Rework requests in execution domain

Rework requests in execution domain

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

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

[Brian Wilkerson, 2015-02-06 02:12:54]

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

[Brian Wilkerson, 2015-02-06 02:12:54]

Addresses issues 22238 and 22239.

[Paul Berry, 2015-02-06 03:55:34]

lgtm, with minor nits.

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/lib/...
File pkg/analysis_server/lib/src/domain_execution.dart (right):

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/domain_execution.dart:118: return new
Response.invalidParameter(request, 'file', 'The file must be a file');
This seems like a confusing error message.  Considering that the message will be
appended to "Invalid parameter 'file'", How about simply saying "Must not refer
to a directory"?

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/domain_execution.dart:129: 'The uri must be a valid
URI');
Similarly, this will be formatted as "Invalid parameter 'uri'.  The uri must be
a valid URI'", which I think people will find redundant :)

How about simply "Invalid URI"?

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

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/test...
pkg/analysis_server/test/domain_execution_test.dart:124: test('does not exist',
() {
Did you copy/paste the test name from the previous test name and forget to
rename it?  I think this test should be called something like "invalid -
directory".

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/test...
pkg/analysis_server/test/domain_execution_test.dart:171: test('both file and
uri', () {
Should we also have a test for the case where neither file nor uri is specified?

[scheglov, 2015-02-06 04:33:13]

LGTM

[Brian Wilkerson, 2015-02-06 15:49:01]

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/lib/...
File pkg/analysis_server/lib/src/domain_execution.dart (right):

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/domain_execution.dart:118: return new
Response.invalidParameter(request, 'file', 'The file must be a file');
Done

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/lib/...
pkg/analysis_server/lib/src/domain_execution.dart:129: 'The uri must be a valid
URI');
Done

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

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/test...
pkg/analysis_server/test/domain_execution_test.dart:124: test('does not exist',
() {
Done

https://codereview.chromium.org/904733003/diff/20001/pkg/analysis_server/test...
pkg/analysis_server/test/domain_execution_test.dart:171: test('both file and
uri', () {
Done

[Brian Wilkerson, 2015-02-06 15:50:32]

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