Change quoting in Process implementation on Windows.

Change quoting in Process implementation on Windows.

Only quote command and arguments if they contain a space or a tab.
This seems consistent with the way it is handled in other systems.

R=antonm@google.com,sgjesse@google.com

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

[Anton Muhin, 2012-03-15 11:52:48]

https://chromiumcodereview.appspot.com/9701061/diff/1/runtime/bin/process_win.cc
File runtime/bin/process_win.cc (right):

https://chromiumcodereview.appspot.com/9701061/diff/1/runtime/bin/process_win...
runtime/bin/process_win.cc:579: char* format = strchr(path, ' ') != NULL ?
"\"%s\"" : "%s";
what if argument is already quoted, do we want to care?

https://chromiumcodereview.appspot.com/9701061/diff/1/runtime/bin/process_win...
runtime/bin/process_win.cc:587: (strchr(arguments[i], '\t') != NULL));
what about other whitespaces?

[Mads Ager (google), 2012-03-15 14:32:22]

Urgh, this is more complicated. We are now escaping in a way such that the
Windows C library will pass on the argument that we actually wanted to pass in.

Moved handling of this to Dart code as it is easier to deal with there.

https://chromiumcodereview.appspot.com/9701061/diff/1/runtime/bin/process_win.cc
File runtime/bin/process_win.cc (right):

https://chromiumcodereview.appspot.com/9701061/diff/1/runtime/bin/process_win...
runtime/bin/process_win.cc:579: char* format = strchr(path, ' ') != NULL ?
"\"%s\"" : "%s";
On 2012/03/15 11:52:48, antonmuhin wrote:
> what if argument is already quoted, do we want to care?

We don't care that the argument is already quoted. However, if we surround the
argument with quotes, we need to escape any quotes in the string:

a " b -> "a \" b"

That will allow the Windows C library arguments parsing to give 'a " b' as one
argument. Similar care should be taken with \ in an argument that we quote.

https://chromiumcodereview.appspot.com/9701061/diff/1/runtime/bin/process_win...
runtime/bin/process_win.cc:587: (strchr(arguments[i], '\t') != NULL));
On 2012/03/15 11:52:48, antonmuhin wrote:
> what about other whitespaces?

In the command-line that you create the arguments are space or tab separated
only.

[Anton Muhin, 2012-03-15 14:59:38]

LGTM

https://chromiumcodereview.appspot.com/9701061/diff/4001/runtime/bin/process_...
File runtime/bin/process_impl.dart (right):

https://chromiumcodereview.appspot.com/9701061/diff/4001/runtime/bin/process_...
runtime/bin/process_impl.dart:63: // twice as many '\' followed by '\"'.
I don't know the current status of RegExps, but it looks like a perfect match
for them, something like:

re.replace('(\+)"', '$1$1\"\).

https://chromiumcodereview.appspot.com/9701061/diff/4001/runtime/bin/process_...
runtime/bin/process_impl.dart:67: while (quotePos != -1) {
absolutely up to you,  but I would structure the loop somewhat differently:

while (true) {
  quotePos = ...
  if (quotePos == -1) {
    finish and break;
  }
  ...
}

[Mads Ager (google), 2012-03-15 15:32:02]

https://chromiumcodereview.appspot.com/9701061/diff/4001/runtime/bin/process_...
File runtime/bin/process_impl.dart (right):

https://chromiumcodereview.appspot.com/9701061/diff/4001/runtime/bin/process_...
runtime/bin/process_impl.dart:63: // twice as many '\' followed by '\"'.
On 2012/03/15 14:59:39, antonmuhin wrote:
> I don't know the current status of RegExps, but it looks like a perfect match
> for them, something like:
> 
> re.replace('(\+)"', '$1$1\"\).

Yeah, it does. I don't believe that there is support for this syntax though.
Therefore you will have to pick out substrings by hand and the code becomes
almost as big. At that point I don't think the regexp machinery is worth it.

https://chromiumcodereview.appspot.com/9701061/diff/4001/runtime/bin/process_...
runtime/bin/process_impl.dart:67: while (quotePos != -1) {
On 2012/03/15 14:59:39, antonmuhin wrote:
> absolutely up to you,  but I would structure the loop somewhat differently:
> 
> while (true) {
>   quotePos = ...
>   if (quotePos == -1) {
>     finish and break;
>   }
>   ...
> }

I don't really like any of them. I like the explicit break condition the best
though, so leaving as is. Thanks.