Fixes for forgotten review comments.

Fixes for forgotten review comments.

Sorry, I forgot to fix the most important review comments from previous CL = https://codereview.chromium.org/16964008/


R=rnystrom@google.com
BUG=

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

[Bob Nystrom, 2013-06-18 15:58:25]

A couple of nits then LGTM.

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
File pkg/analyzer_experimental/bin/coverage.dart (right):

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
pkg/analyzer_experimental/bin/coverage.dart:84: print('$usage\n\n');
print() will add one newline for you, so you don't need the double ones anymore.

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
File
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart
(right):

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart:37:
.then((Process process) {
If you want, you can move this .then() call one level up. We tend to do that to
flatten out a series of async operations, like:

.then((port) {
  ...
  return Process.start(...);
}).then((process) {
  ...
  return process.exitCode;
}).catchError((e) {
  ...
});

The semantics are the same either way, but this way it's a little less nested.

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart:126:
path = pathos.joinAll(parts);
The "Builder" name is a bit confusing in pathos, but you should use the same
builder object for these method calls too. Basically, anywhere you're doing
"pathos." use the builder instead. That way you control the style (URL) that it
uses.

[scheglov, 2013-06-18 16:36:11]

PTAL

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
File pkg/analyzer_experimental/bin/coverage.dart (right):

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
pkg/analyzer_experimental/bin/coverage.dart:84: print('$usage\n\n');
On 2013/06/18 15:58:25, Bob Nystrom wrote:
> print() will add one newline for you, so you don't need the double ones
anymore.

Done.

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
File
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart
(right):

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart:37:
.then((Process process) {
On 2013/06/18 15:58:25, Bob Nystrom wrote:
> If you want, you can move this .then() call one level up. We tend to do that
to
> flatten out a series of async operations, like:
> 
> .then((port) {
>   ...
>   return Process.start(...);
> }).then((process) {
>   ...
>   return process.exitCode;
> }).catchError((e) {
>   ...
> });
> 
> The semantics are the same either way, but this way it's a little less nested.

Done.

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart:126:
path = pathos.joinAll(parts);
On 2013/06/18 15:58:25, Bob Nystrom wrote:
> The "Builder" name is a bit confusing in pathos, but you should use the same
> builder object for these method calls too. Basically, anywhere you're doing
> "pathos." use the builder instead. That way you control the style (URL) that
it
> uses.

Hm...
I use URL style to convert URI to path parts.
But after this, I want to work with file system paths, so this is why I use
"pathos" functions.
Actually... I don't need even Builder to split the URI path, it is always '/',
right?
Or, even better - use Uri.pathSegments :-)

[Bob Nystrom, 2013-06-18 17:22:27]

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
File
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart
(right):

https://codereview.chromium.org/16885003/diff/2002/pkg/analyzer_experimental/...
pkg/analyzer_experimental/lib/src/services/runtime/coverage/coverage_impl.dart:126:
path = pathos.joinAll(parts);
On 2013/06/18 16:36:11, scheglov wrote:
> On 2013/06/18 15:58:25, Bob Nystrom wrote:
> > The "Builder" name is a bit confusing in pathos, but you should use the same
> > builder object for these method calls too. Basically, anywhere you're doing
> > "pathos." use the builder instead. That way you control the style (URL) that
> it
> > uses.
> 
> Hm...
> I use URL style to convert URI to path parts.
> But after this, I want to work with file system paths, so this is why I use
> "pathos" functions.

Oh, sorry, right. I wasn't reading the code closely.

> Actually... I don't need even Builder to split the URI path, it is always '/',
> right?
> Or, even better - use Uri.pathSegments :-)

Yes! :)

[scheglov, 2013-06-18 18:38:17]

Committed patchset #4 manually as r24157 (presubmit successful).