Allow typing label names on Windows by not treating things that start with slashes as arguments rat…

Allow typing label names on Windows by not treating things that start with slashes as arguments rather than switches.

The old command line parsing is still run so some code could get confused later since / will also be treated as a switch if you ask for it. This seems not as likely to happen that the label will match a switch. If it does, we'll have to change the CommandLine class to make the "/" behavior optional.

Fix a "desc" crash attributing blame due to a null pointer.

BUG=
R=scottmg@chromium.org, thakis@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=225277

[scottmg, 2013-09-25 17:23:23]

lgtm

https://codereview.chromium.org/24613003/diff/1/tools/gn/gn_main.cc
File tools/gn/gn_main.cc (right):

https://codereview.chromium.org/24613003/diff/1/tools/gn/gn_main.cc#newcode23
tools/gn/gn_main.cc:23: // switches. This makes it impossible to type label
names like "//foo:bar".
seems like it'd be a useful option for CommandLine

[brettw, 2013-09-25 18:14:19]

thakis for base changes:

I went back and forth about whether "slash is a switch" should be a property of
a command line or a global property of the app. A global seems hacky, but the
per-class value isn't obviously more correct. The plumbing for the per-class
implementation would have been pretty messy, so I opted for the global setting
solution.

[scottmg, 2013-09-25 18:20:35]

gn lgtm

https://codereview.chromium.org/24613003/diff/6001/base/command_line.cc
File base/command_line.cc (right):

https://codereview.chromium.org/24613003/diff/6001/base/command_line.cc#newco...
base/command_line.cc:38: size_t switch_prefix_count =
arraysize(kSwitchPrefixes);
nit; don't see a lot of benefit to the const for non-windows.

[Nico, 2013-09-25 20:08:16]

Huh, slashes not being switches is pretty surprising on Windows, no? Isn't there
some other way to fix this? Like e.g. the '--' thing on posix (`program -flag --
-not_a_flag_since_its_behind_dashdash`)? Or by quoting the slash, or by allowing
"!" as a synonym for "/" n gn for labels?

[scottmg, 2013-09-25 20:18:00]

On 2013/09/25 20:08:16, Nico wrote:
> Huh, slashes not being switches is pretty surprising on Windows, no? Isn't
there
> some other way to fix this? Like e.g. the '--' thing on posix (`program -flag
--
> -not_a_flag_since_its_behind_dashdash`)? Or by quoting the slash, or by
allowing
> "!" as a synonym for "/" n gn for labels?

I wouldn't personally miss them, but a bit surprising I guess.

It's a bit less suitable for inclusion in base, but maybe // could just be
treated specially. i.e. / still starts options, but // doesn't. It seems
sacrificable to lose the ability to parse an option name that also starts with a
/, which presumably is very uncommon.

[brettw, 2013-09-25 21:16:27]

Nobody is going to remember a !! synonym and I agree special casing "//" is not
good for base. I'm a windows person and not supporting "/" options isn't
surprising to me, especially when the help says to use various hyphens.

If you don't like this change I would prefer patch set 1 than hacking around it
in other ways.

[Nico, 2013-09-25 21:23:26]

On Wed, Sep 25, 2013 at 2:16 PM, <brettw@chromium.org> wrote:

> Nobody is going to remember a !! synonym and I agree special casing "//"
> is not
> good for base. I'm a windows person and not supporting "/" options isn't
> surprising to me, especially when the help says to use various hyphens.
>
> If you don't like this change I would prefer patch set 1 than hacking
> around it
> in other ways.
>

I hadn't seen patch set 1. I think I weakly prefer that, but since both you
and Scott don't think '/' not starting a command one windows is surprising,
this lgtm too.


>
>
https://codereview.chromium.**org/24613003/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[brettw, 2013-09-25 23:36:11]

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