Add optional workingDirectory argument to Process.start.

Add optional workingDirectory argument to Process.start.
Committed: https://code.google.com/p/dart/source/detail?r=2987

[Søren Gjesse, 2012-01-05 09:30:47]

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.cc
File runtime/bin/process.cc (right):

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.cc#newcode57
runtime/bin/process.cc:57: // Default to the current working directoy.
Default => Defaults

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.dart
File runtime/bin/process.dart (right):

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.dart#newcode12
runtime/bin/process.dart:12: * from.
I think there should be some additional info on the use of relative paths here.
On Linux and Mac OS the change of working directory happens before locating the
executable whereas from the Windows documentation of CreateProcess it seems that
that might not be the case. Maybe just a comment on that combining relative
paths for the executable with a working directory might work differently on
different platforms.

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.dart#newcode17
runtime/bin/process.dart:17: Process.start(String path, List<String> arguments,
[String workingDirectory]);
Maybe we should rename path to executable.

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process_impl.dart
File runtime/bin/process_impl.dart (right):

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process_impl.dart#n...
runtime/bin/process_impl.dart:13: _Process.start(String path, List<String>
arguments, [String workingDirectory]) {
Ling line, do either

_Process.start(
    String path, List<String> arguments, [String workingDirectory]) {
  ...
}

or

_Process.start(String path,
               List<String> arguments,
               [String workingDirectory]) {
  ...
}

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process_impl.dart#n...
runtime/bin/process_impl.dart:60: bool success = _start(
When all arguments cannot fit on one line they should each be on separate lines.

It is not mandated by
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...,
but we usually do.

[Anders Johnsen, 2012-01-05 09:50:49]

Fixed

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.cc
File runtime/bin/process.cc (right):

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.cc#newcode57
runtime/bin/process.cc:57: // Default to the current working directoy.
On 2012/01/05 09:30:47, Søren Gjesse wrote:
> Default => Defaults

Done.

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.dart
File runtime/bin/process.dart (right):

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.dart#newcode12
runtime/bin/process.dart:12: * from.
On 2012/01/05 09:30:47, Søren Gjesse wrote:
> I think there should be some additional info on the use of relative paths
here.
> On Linux and Mac OS the change of working directory happens before locating
the
> executable whereas from the Windows documentation of CreateProcess it seems
that
> that might not be the case. Maybe just a comment on that combining relative
> paths for the executable with a working directory might work differently on
> different platforms.

Nice observation, added further comments.

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process.dart#newcode17
runtime/bin/process.dart:17: Process.start(String path, List<String> arguments,
[String workingDirectory]);
On 2012/01/05 09:30:47, Søren Gjesse wrote:
> Maybe we should rename path to executable.

Agreed.

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process_impl.dart
File runtime/bin/process_impl.dart (right):

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process_impl.dart#n...
runtime/bin/process_impl.dart:13: _Process.start(String path, List<String>
arguments, [String workingDirectory]) {
On 2012/01/05 09:30:47, Søren Gjesse wrote:
> Ling line, do either
> 
> _Process.start(
>     String path, List<String> arguments, [String workingDirectory]) {
>   ...
> }
> 
> or
> 
> _Process.start(String path,
>                List<String> arguments,
>                [String workingDirectory]) {
>   ...
> }

Done.

http://codereview.chromium.org/9108008/diff/1/runtime/bin/process_impl.dart#n...
runtime/bin/process_impl.dart:60: bool success = _start(
On 2012/01/05 09:30:47, Søren Gjesse wrote:
> When all arguments cannot fit on one line they should each be on separate
lines.
> 
> It is not mandated by
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Functi...,
> but we usually do.

Nice to know, ty!

[Søren Gjesse, 2012-01-05 09:54:08]

lgtm

[Mads Ager (google), 2012-01-05 10:09:21]

Drive-by nit. :)

http://codereview.chromium.org/9108008/diff/1011/tests/standalone/src/Process...
File tests/standalone/src/ProcessWorkingDirectoryTest.dart (right):

http://codereview.chromium.org/9108008/diff/1011/tests/standalone/src/Process...
tests/standalone/src/ProcessWorkingDirectoryTest.dart:23:
print(fullTestFilePath);
Please remove this debug print.