first cut simple code completion results

first cut simple code completion results
- not all CompletionSuggestions implemented yet
- results valid in only a few situations
- more to come

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

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

[danrubel, 2014-07-17 19:57:36]

Incomplete implementation, but want to land something simple on which I can
further implement.

[scheglov, 2014-07-17 20:40:00]

lgtm

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
File pkg/analysis_server/lib/src/constants.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/constants.dart:85: const String COMPLETION =
'completion';
It should be before any CON* names.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
File pkg/analysis_server/lib/src/domain_completion.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:31: * The next searc response
id.
The comment has couple nits: "searc" is an incomplete word; and it is not a
search :-)

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:55: RequestDatum fileDatum =
request.getRequiredParameter(FILE);
You could include these Datum objects here and later, they are pretty much
noise.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:71: void
_sendSearchNotification(String completionId, bool isLast,
_secondCompletionNotification?

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:84: class
TopLevelSuggestionsComputer {
Make it private?

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:93: var future =
searchEngine.searchTopLevelDeclarations('');
Should we use an existing prefix here?

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
pkg/analysis_server/test/completion_test.dart:48: Future getSuggestions(String
pattern, int offset) {
You don't use "pattern", did you mean to find offset using it?

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
pkg/analysis_server/test/completion_test.dart:92: import 'dart:html';
It's up to you, but I'd start sample code with line starts, it is easier to
write this way.
And it looks almost as a normal code.

[Brian Wilkerson, 2014-07-17 20:41:52]

LGTM

[danrubel, 2014-07-17 21:01:14]

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
File pkg/analysis_server/lib/src/constants.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/constants.dart:85: const String COMPLETION =
'completion';
On 2014/07/17 20:39:59, scheglov wrote:
> It should be before any CON* names.

Done.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
File pkg/analysis_server/lib/src/domain_completion.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:31: * The next searc response
id.
On 2014/07/17 20:40:00, scheglov wrote:
> The comment has couple nits: "searc" is an incomplete word; and it is not a
> search :-)

Done.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:55: RequestDatum fileDatum =
request.getRequiredParameter(FILE);
On 2014/07/17 20:40:00, scheglov wrote:
> You could include these Datum objects here and later, they are pretty much
> noise.

Good point. Done.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:71: void
_sendSearchNotification(String completionId, bool isLast,
On 2014/07/17 20:39:59, scheglov wrote:
> _secondCompletionNotification?

Done.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:84: class
TopLevelSuggestionsComputer {
On 2014/07/17 20:40:00, scheglov wrote:
> Make it private?

I didn't bother because I envision more of these and moving this one out into
its own file similar to the other computers you created.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:93: var future =
searchEngine.searchTopLevelDeclarations('');
On 2014/07/17 20:40:00, scheglov wrote:
> Should we use an existing prefix here?

I want to return the complete list and let the client filter it. The user may
have typed/deleted characters by the time the notification is received by the
client. If the user is still typing/deleting within the same identifier, then I
want the results to still be usable by the client. No need for another round
trip to the server.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
pkg/analysis_server/test/completion_test.dart:48: Future getSuggestions(String
pattern, int offset) {
On 2014/07/17 20:40:00, scheglov wrote:
> You don't use "pattern", did you mean to find offset using it?

Yes! Good eyes. Fixed.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
pkg/analysis_server/test/completion_test.dart:92: import 'dart:html';
On 2014/07/17 20:40:00, scheglov wrote:
> It's up to you, but I'd start sample code with line starts, it is easier to
> write this way.
> And it looks almost as a normal code.

I found it jarring to see things abruptly left aligned.
I find it easier to read this way.

[scheglov, 2014-07-17 21:09:32]

LGTM

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
File pkg/analysis_server/lib/src/domain_completion.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/lib/src/...
pkg/analysis_server/lib/src/domain_completion.dart:93: var future =
searchEngine.searchTopLevelDeclarations('');
On 2014/07/17 21:01:14, danrubel wrote:
> On 2014/07/17 20:40:00, scheglov wrote:
> > Should we use an existing prefix here?
> 
> I want to return the complete list and let the client filter it. The user may
> have typed/deleted characters by the time the notification is received by the
> client. If the user is still typing/deleting within the same identifier, then
I
> want the results to still be usable by the client. No need for another round
> trip to the server.

OK, good point.

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/402723003/diff/1/pkg/analysis_server/test/com...
pkg/analysis_server/test/completion_test.dart:92: import 'dart:html';
On 2014/07/17 21:01:14, danrubel wrote:
> On 2014/07/17 20:40:00, scheglov wrote:
> > It's up to you, but I'd start sample code with line starts, it is easier to
> > write this way.
> > And it looks almost as a normal code.
> 
> I found it jarring to see things abruptly left aligned.
> I find it easier to read this way.

OK

https://codereview.chromium.org/402723003/diff/20001/pkg/analysis_server/test...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/402723003/diff/20001/pkg/analysis_server/test...
pkg/analysis_server/test/completion_test.dart:50: int offset =
testCode.indexOf(pattern) + offsetFromPatternStart;
Would it be easier to use some special character, like ^ or ! as a marker for
the position to request completions? So, it'll be easier to write tests.

We could override addTestFile() in this class to find the completion offset and
remove the special character from the code.

[danrubel, 2014-07-17 21:11:44]

Committed patchset #3 manually as r38347 (presubmit successful).

[danrubel, 2014-07-17 21:38:45]

https://codereview.chromium.org/402723003/diff/20001/pkg/analysis_server/test...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/402723003/diff/20001/pkg/analysis_server/test...
pkg/analysis_server/test/completion_test.dart:50: int offset =
testCode.indexOf(pattern) + offsetFromPatternStart;
On 2014/07/17 21:09:32, scheglov wrote:
> Would it be easier to use some special character, like ^ or ! as a marker for
> the position to request completions? So, it'll be easier to write tests.
> 
> We could override addTestFile() in this class to find the completion offset
and
> remove the special character from the code.

Great suggestion. https://codereview.chromium.org/402803004/