[PATCH] dispatch: strip command line options like config file options

[Yuya Nishihara]

On Fri, 12 Feb 2016 11:48:44 +0000, Pierre-Yves David wrote:
> On 02/11/2016 11:25 PM, Tony Tung wrote:
> >> On Feb 11, 2016, at 2:09 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> >> On Thu, 11 Feb 2016 08:06:40 +0000, Tony Tung wrote:
> >>>> On Feb 10, 2016, at 11:49 PM, Yuya Nishihara <yuya at tcha.org> wrote:
> >>>>
> >>>> On Mon, 8 Feb 2016 15:36:15 -0800, Tony Tung wrote:
> >>>>> # HG changeset patch
> >>>>> # User Tony Tung <ttung at fb.com>
> >>>>> # Date 1454974530 28800
> >>>>> #      Mon Feb 08 15:35:30 2016 -0800
> >>>>> # Node ID 97240c9addb84f73752dad57e6531dfea30f3f0d
> >>>>> # Parent  01a5143cd25f285f8c745a92986cd7186bb32c90
> >>>>> dispatch: strip command line options like config file options.
> >>>>>
> >>>>> Currently, whitespace in command line --config options are considered
> >>>>> significant while whitespace in config files are not considered
> >>>>> significant.  This diff strips the leading and trailing whitespace from
> >>>>> command line config options.
> >>>>>
> >>>>> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> >>>>> --- a/mercurial/dispatch.py
> >>>>> +++ b/mercurial/dispatch.py
> >>>>> @@ -609,7 +609,8 @@
> >>>>>
> >>>>>     for cfg in config:
> >>>>>         try:
> >>>>> -            name, value = cfg.split('=', 1)
> >>>>> +            name, value = [cfgelem.strip()
> >>>>> +                           for cfgelem in cfg.split('=', 1)]
> >>>>
> >>>> I think that's up to parser whether white space is significant or not.
> >>>
> >>> The parser definitely has a say into the significance of the outer
> >>> whitespaces, but it has no visibility into the inner whitespaces,
> >>> i.e., the spaces surrounding the =.
> >>
> >> I'm not sure if I understand what you mean. I wanted to tell that
> >> --config is parsed as a command-line option, which rule can be different
> >> from a config file. And if --config contains a whitespace, it would be
> >> considered explicitly specified.
> >>
> >>   --config foo.bar="baz "
> >>
> >> That said, because config key shouldn't start/end with space,
> >> --config " foo . bar "=baz might be an error.
> >
> > Why is explicit whitespace in a config file treated differently than explicit whitespace in the command line?
> 
> I think that what Yuya means here is:
> 
> - white space stripping is handled by the .ini file parsing,
> - that .ini parsing is third party code with probably non-trivial logic,
> - We (Yuya, I, etc) are not certain what these .ini parsing rules are,
> - Therefor we are not confident that "simply stripping white space" from 
> the command line will give us a behavior consistent with "what happen in 
> the config file"

We have own config parser. My opinion is that it is common to strip
whitespaces while parsing ini-style file, but it isn't for command option.

That said, it appears I'm wrong. gcc does it.

  % echo 'FOO' | gcc -E -D" FOO = a " -
  a

https://github.com/gcc-mirror/gcc/blob/gcc_5_3_0_release/libcpp/directives.c#L2361

So, I'm inclined to accepting this patch.