Add API to provide a selection range, and return the updated selection after formatting.

Add API to provide a selection range, and return the updated selection after formatting.

Fix #93.

BUG=https://github.com/dart-lang/dart_style/issues/93
R=brianwilkerson@google.com

Committed: 119b00b06d1ecde69e01859413329e32b6cd6c14

[Bob Nystrom, 2015-01-07 23:10:41]

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

[Bob Nystrom, 2015-01-07 23:10:42]

After this lands, to format some code with a selection, you basically do:

var sourceCode = new SourceCode(someCode,
    selectionStart: ...
    selectionLength: ...);

var result = new DartFormatter().formatSource(sourceCode);
result.text; // formatted code
result.selectionStart; // updated selection
result.selectionLength; // updated length

There's one corner of the behavior that's well-defined and tested but maybe not
ideal. The formatter has to decide how to handle the selection starting or
ending inside whitespace. Right now, it pushes both the start and end to the
right to the next non-whitespace character.

I think that's the right behavior for the selection start, but I feel like
pushing to the left is probably better for the end. That would mean that the
updated selection basically shrinks to only contain the non-whitespace stuff you
previously had selected.

But I'm not sure, and doing that will add a bit of complexity, so I didn't do it
for now. What do you think?

- bob

[Brian Wilkerson, 2015-01-08 00:17:45]

LGTM!

I think you're right that it makes sense to push the selection start to the
right and the end to the left. Except, of course, if the selection covers only
white space between two tokens, in which case that heuristic would result in a
negative selection. :-) Another special case might be a selection within a
comment, in which case it would be nice if the selection could remain the same
except for offsetting the start by the amount that the comment was moved
(assuming that you're not reformatting the content of comments). But you're
definitely right that this is complex.

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
File lib/src/dart_formatter.dart (right):

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
lib/src/dart_formatter.dart:54: if (selectionLength == null) {
I think this case is covered by line 40.

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
lib/src/dart_formatter.dart:153: var parser = new Parser(stringSource,
errorListener);
It would be nice (in a later CL) if there were a way to pass in an already
computed AST structure when one is available.

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
lib/src/dart_formatter.dart:154: parser.parseAsync = true;
This should no longer be necessary. Async support is now always enabled.

[Bob Nystrom, 2015-01-08 00:36:38]

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
File lib/src/dart_formatter.dart (right):

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
lib/src/dart_formatter.dart:54: if (selectionLength == null) {
On 2015/01/08 00:17:45, Brian Wilkerson wrote:
> I think this case is covered by line 40.

Oops, forgot to delete this! Done.

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
lib/src/dart_formatter.dart:153: var parser = new Parser(stringSource,
errorListener);
On 2015/01/08 00:17:45, Brian Wilkerson wrote:
> It would be nice (in a later CL) if there were a way to pass in an already
> computed AST structure when one is available.

Yup. Filed a tracking bug: https://github.com/dart-lang/dart_style/issues/123

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
lib/src/dart_formatter.dart:154: parser.parseAsync = true;
On 2015/01/08 00:17:45, Brian Wilkerson wrote:
> This should no longer be necessary. Async support is now always enabled.

dart_style is using this constraint for the analyzer:

  analyzer: '>=0.22.0 <0.23.0'

So it doesn't include any of the latest versions since they are "-dev"
prerelease versions. With 0.22.4, I do need to set parseAsync. Should I bump
dart_style to newer versions of the analyzer? What constraint should I use?

[Bob Nystrom, 2015-01-08 00:36:59]

Committed patchset #2 (id:20001) manually as
119b00b06d1ecde69e01859413329e32b6cd6c14 (presubmit successful).

[pquitslund, 2015-01-08 00:39:25]

LGTM.  Sweet! :)

[Bob Nystrom, 2015-01-08 00:45:01]

On 2015/01/08 00:17:45, Brian Wilkerson wrote:
> LGTM!
> 
> I think you're right that it makes sense to push the selection start to the
> right and the end to the left. Except, of course, if the selection covers only
> white space between two tokens, in which case that heuristic would result in a
> negative selection. :-)

Filed a bug: https://github.com/dart-lang/dart_style/issues/124

This won't be a breaking change, so I think it makes sense to fix it separately
so you guys aren't blocked.

> Another special case might be a selection within a
> comment, in which case it would be nice if the selection could remain the same
> except for offsetting the start by the amount that the comment was moved
> (assuming that you're not reformatting the content of comments). But you're
> definitely right that this is complex.

Comments should work correctly now. From the formatter's perspective, they're as
"real" as code, and if the selection is in the middle of a comment, it's
relative location should be preserved even when the comment moves around.

Cheers!

- bob

[Brian Wilkerson, 2015-01-08 05:31:29]

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
File lib/src/dart_formatter.dart (right):

https://chromiumcodereview.appspot.com/822273004/diff/1/lib/src/dart_formatte...
lib/src/dart_formatter.dart:154: parser.parseAsync = true;
> With 0.22.4, I do need to set parseAsync.

In that case, you probably also want to add:

  parser.parseDeferredLibraries = true;
  parser.parseEnum = true;