Cleaned up the SDEF...

Cleaned up the SDEF
Added execute javascript command
Updated tests
Fixed a bug in tabapplescript
Added more examples

[pink (ping after 24hrs), 2010-08-13 21:06:27]

mostly lgtm, a few formatting comments.

http://codereview.chromium.org/3180006/diff/1/3
File chrome/browser/cocoa/applescript/examples/copy_html.applescript (right):

http://codereview.chromium.org/3180006/diff/1/3#newcode7
chrome/browser/cocoa/applescript/examples/copy_html.applescript:7: tell tab 1 of
window 1 to view source
the previous file had 2-space indents. can you be consistent at least?

http://codereview.chromium.org/3180006/diff/1/4
File chrome/browser/cocoa/applescript/examples/delete_bookmarks.applescript
(right):

http://codereview.chromium.org/3180006/diff/1/4#newcode12
chrome/browser/cocoa/applescript/examples/delete_bookmarks.applescript:12: end
tell
should this line up with the tell above it, and not the set?

http://codereview.chromium.org/3180006/diff/1/8
File chrome/browser/cocoa/applescript/scripting.sdef (right):

http://codereview.chromium.org/3180006/diff/1/8#newcode262
chrome/browser/cocoa/applescript/scripting.sdef:262: <direct-parameter
description="The tab to execute the command in." type="specifier"/>
"The tab in which to execute the command". Don't end sentences with prepositions
:-)

[shreyas, 2010-08-13 21:18:58]

http://codereview.chromium.org/3180006/diff/1/3
File chrome/browser/cocoa/applescript/examples/copy_html.applescript (right):

http://codereview.chromium.org/3180006/diff/1/3#newcode7
chrome/browser/cocoa/applescript/examples/copy_html.applescript:7: tell tab 1 of
window 1 to view source
I am not entering a new scope, when you append |to| at the end of a tell command
the scope ends there.
On 2010/08/13 21:06:27, pink wrote:
> the previous file had 2-space indents. can you be consistent at least?

http://codereview.chromium.org/3180006/diff/1/8
File chrome/browser/cocoa/applescript/scripting.sdef (right):

http://codereview.chromium.org/3180006/diff/1/8#newcode262
chrome/browser/cocoa/applescript/scripting.sdef:262: <direct-parameter
description="The tab to execute the command in." type="specifier"/>
Apple use the exact same style, was trying to be consistent.
On 2010/08/13 21:06:27, pink wrote:
> "The tab in which to execute the command". Don't end sentences with
prepositions
> :-)

[Bons, 2010-08-13 21:44:27]

http://codereview.chromium.org/3180006/diff/12001/13002
File chrome/browser/cocoa/applescript/examples/copy_html.applescript (right):

http://codereview.chromium.org/3180006/diff/12001/13002#newcode7
chrome/browser/cocoa/applescript/examples/copy_html.applescript:7: tell tab 1 of
window 1 to view source
indentation should be consistent across files.

http://codereview.chromium.org/3180006/diff/12001/13008
File chrome/browser/cocoa/applescript/tab_applescript.h (right):

http://codereview.chromium.org/3180006/diff/12001/13008#newcode74
chrome/browser/cocoa/applescript/tab_applescript.h:74: // Executes a piece of
javscript in the tab.
s/javascript/Javascript

http://codereview.chromium.org/3180006/diff/12001/13009
File chrome/browser/cocoa/applescript/tab_applescript.mm (right):

http://codereview.chromium.org/3180006/diff/12001/13009#newcode33
chrome/browser/cocoa/applescript/tab_applescript.mm:33: SessionID::id_type
futureSessionIDOfTab = session.id() + 1;
Why the +1? Because AS is 1-based instead of 0-based?

http://codereview.chromium.org/3180006/diff/12001/13010
File chrome/browser/cocoa/applescript/window_applescript_test.mm (right):

http://codereview.chromium.org/3180006/diff/12001/13010#newcode116
chrome/browser/cocoa/applescript/window_applescript_test.mm:116: [tab
containerProperty]);
this seems like it would fit on the previous line.

[shreyas, 2010-08-13 22:30:58]

http://codereview.chromium.org/3180006/diff/12001/13002
File chrome/browser/cocoa/applescript/examples/copy_html.applescript (right):

http://codereview.chromium.org/3180006/diff/12001/13002#newcode7
chrome/browser/cocoa/applescript/examples/copy_html.applescript:7: tell tab 1 of
window 1 to view source
I am not entering a new scope, when you append |to| at the end of a tell command
the scope ends there.
On 2010/08/13 21:44:27, Andrew Bonventre (Bons) wrote:
> indentation should be consistent across files.

http://codereview.chromium.org/3180006/diff/12001/13009
File chrome/browser/cocoa/applescript/tab_applescript.mm (right):

http://codereview.chromium.org/3180006/diff/12001/13009#newcode33
chrome/browser/cocoa/applescript/tab_applescript.mm:33: SessionID::id_type
futureSessionIDOfTab = session.id() + 1;
No, because the new tab that will be created will have this.
The problem is i am dealing with API's where to create a tab, i need to know the
window, this leads to temp values and knowing the next id.
On 2010/08/13 21:44:27, Andrew Bonventre (Bons) wrote:
> Why the +1? Because AS is 1-based instead of 0-based?

http://codereview.chromium.org/3180006/diff/12001/13010
File chrome/browser/cocoa/applescript/window_applescript_test.mm (right):

http://codereview.chromium.org/3180006/diff/12001/13010#newcode116
chrome/browser/cocoa/applescript/window_applescript_test.mm:116: [tab
containerProperty]);
Wow, thats a sharp eye.
On 2010/08/13 21:44:27, Andrew Bonventre (Bons) wrote:
> this seems like it would fit on the previous line.

[andybons, 2010-08-13 22:40:20]

Lgtm

Regarding the +1 issue, can you add a comment about it?

A

On Aug 13, 2010, at 18:30, v.a.shreyas@gmail.com wrote:

>
> http://codereview.chromium.org/3180006/diff/12001/13002
> File chrome/browser/cocoa/applescript/examples/copy_html.applescript
> (right):
>
> http://codereview.chromium.org/3180006/diff/12001/13002#newcode7
> chrome/browser/cocoa/applescript/examples/copy_html.applescript:7: tell
> tab 1 of window 1 to view source
> I am not entering a new scope, when you append |to| at the end of a tell
> command
> the scope ends there.
> On 2010/08/13 21:44:27, Andrew Bonventre (Bons) wrote:
>> indentation should be consistent across files.
>
> http://codereview.chromium.org/3180006/diff/12001/13009
> File chrome/browser/cocoa/applescript/tab_applescript.mm (right):
>
> http://codereview.chromium.org/3180006/diff/12001/13009#newcode33
> chrome/browser/cocoa/applescript/tab_applescript.mm:33:
> SessionID::id_type futureSessionIDOfTab = session.id() + 1;
> No, because the new tab that will be created will have this.
> The problem is i am dealing with API's where to create a tab, i need to
> know the window, this leads to temp values and knowing the next id.
> On 2010/08/13 21:44:27, Andrew Bonventre (Bons) wrote:
>> Why the +1? Because AS is 1-based instead of 0-based?
>
> http://codereview.chromium.org/3180006/diff/12001/13010
> File chrome/browser/cocoa/applescript/window_applescript_test.mm
> (right):
>
> http://codereview.chromium.org/3180006/diff/12001/13010#newcode116
> chrome/browser/cocoa/applescript/window_applescript_test.mm:116: [tab
> containerProperty]);
> Wow, thats a sharp eye.
> On 2010/08/13 21:44:27, Andrew Bonventre (Bons) wrote:
>> this seems like it would fit on the previous line.
>
> http://codereview.chromium.org/3180006/show

[shreyas, 2010-08-17 13:41:20]

have a look at it.

[pink (ping after 24hrs), 2010-08-17 13:53:04]

FYI, i'll take care of landing this. Sending to the try servers now.

On Tue, Aug 17, 2010 at 9:41 AM,  <v.a.shreyas@gmail.com> wrote:
> have a look at it.
>
> http://codereview.chromium.org/3180006/show
>



-- 
Mike Pinkerton
Mac Weenie
pinkerton@google.com