Formatter service API tweaks.

Formatter service API tweaks.

Some way to indicate no selection is required in the case where there is none.  I opted for the -1 convention not becuase I particularly like it but because, well, it's convention! :)

R=brianwilkerson@google.com, scheglov@google.com

Committed: https://code.google.com/p/dart/source/detail?r=43369

[pquitslund, 2015-02-02 21:43:35]

pquitslund@google.com changed reviewers:
+ brianwilkerson@google.com, scheglov@google.com

[scheglov, 2015-02-02 21:53:57]

LGTM

[Brian Wilkerson, 2015-02-02 22:12:10]

I'm not sure I know what you mean by "no selection". If you mean that the client
is not interested in passing in or getting back the selection information, then
the standard approach in the server API is to mark the field(s) as being
optional.

If you mark the fields as being optional, then you should include text in the
request description indicating that the fields must either both be included or
both be excluded. The result fields should also be marked as being optional with
text explaining that they will be omitted if the corresponding request field was
omitted.

Unfortunately, marking the result fields as being optional is not a backward
compatible change, so we'll need to bump the version to 2.0.0, both in the
document and in the code (AnalysisServer.VERSION).

If you mean that there is literally no selection in the file, then I would think
that an offset and length of zero would be reasonable. (Clients could also pass
in zero for both fields if they're not interested in preserving the selection,
and just ignore the result fields, without requiring a change to the API.)

[pquitslund, 2015-02-02 22:21:59]

On 2015/02/02 22:12:10, Brian Wilkerson wrote:
> If you mean that there is literally no selection in the file, 

Exactly.  For example, when you right-click a file in the explorer.

> then I would think
> that an offset and length of zero would be reasonable. (Clients could also
pass
> in zero for both fields if they're not interested in preserving the selection,
> and just ignore the result fields, without requiring a change to the API.)

Sure.  That being said, I still think the API documentation needs to be updated
to specify that 0 (or -1) indicates no selection.  Otherwise, how is the client
to know?

I would prefer not to make the fields optional for the reasons you describe.  I
don't think using 0 or -1 and updating the doc is too onerous.  Your call.

How about I change it to 0?

[Brian Wilkerson, 2015-02-02 22:34:06]

> ... I still think the API documentation needs to be updated
> to specify that 0 (or -1) indicates no selection.  Otherwise, how is the
client
> to know?

Makes sense. I just hadn't thought of that use case.

> How about I change it to 0?

Sounds good. Then we don't have to special case anything in the code.

Perhaps, rather than including the text in both field descriptions, just a note
in the request description to the effect that clients can use zero for both
fields if they do not care about preserving selection information (such as when
the file is not open and hence there is no selection). (Sorry if I'm being
picky. I'm concerned that if "no selection" confused me then it might confuse
someone else.)

Other than that, LGTM.

[pquitslund, 2015-02-02 23:08:04]

On 2015/02/02 22:34:06, Brian Wilkerson wrote:
> > ... I still think the API documentation needs to be updated
> > to specify that 0 (or -1) indicates no selection.  Otherwise, how is the
> client
> > to know?
> 
> Makes sense. I just hadn't thought of that use case.
> 
> > How about I change it to 0?
> 
> Sounds good. Then we don't have to special case anything in the code.
> 
> Perhaps, rather than including the text in both field descriptions, just a
note
> in the request description to the effect that clients can use zero for both
> fields if they do not care about preserving selection information (such as
when
> the file is not open and hence there is no selection). (Sorry if I'm being
> picky. I'm concerned that if "no selection" confused me then it might confuse
> someone else.)
> 
> Other than that, LGTM.

0 works for me.  Updating.

Thanks for the back and forth!

[pquitslund, 2015-02-02 23:14:30]

Committed patchset #3 (id:40001) manually as r43369 (presubmit successful).