Re: [PATCH 2/3] parse-options: allow positivation of options starting, with no-

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 27.02.2012 18:18, schrieb Junio C Hamano:
Thomas Rast<trast@xxxxxxxxxxx>  writes:

Junio C Hamano<gitster@xxxxxxxxx>  writes:

I would naïvely expect that it would be sufficient to update an existing
definition for "--no-frotz" that uses PARSE_OPT_NONEG to instead define
"--frotz" that by itself is a no-op, and "--no-frotz" would cause whatever
the option currently means, with an update to the help text that says
something to the effect that "--frotz by itself is meaningless and is
always used as --no-frotz".

Doesn't that last quote already answer your question?

Yes, but only partly.  I would agree with the awkwardness in

It would be rather awkward to see, in 'git apply -h',

   --add                 Also apply additions in the patch.  This is the
                         default; use --no-add to disable it.

but it feeels somewhat questionable that the solution to get this:


Compare to the current concise wording

   --no-add              ignore additions made by the patch

is to define OPT_BOOL("no-add") that does not have any hint (other than
the fact that the option name begins with 3 character "no-") that this is
an already negated boolean and the "no-" negation can be removed.

The parser already knows that the prefix "no-" negates an option. It currenmtly only applies this knowledge if that prefix is added, but not if it is removed, which is inconsistent.

This means an option "no-$foo" can never mean anything but "not foo".  Not
that we would have to or necessarily want to support an option to give the
number of foo as --no-foo=47, as --num-foo=47 is a perfectly good spelling
for such an option.

With the patch, you can define a --no-foo option that doesn't accept --foo as its negation by specifying PARSE_OPT_NONEG. That would also forbid --no-no-foo, though, but that's probably a good thing.

If it were OPT_BOOL("no-foo", OPT_ISNEG | ...) that signals the parser
that:

  - the option name is already negative;
  - the leading "no-" is to be removed to negate it; and
  - no extra leading "no-", i.e. "--no-no-foo", is accepted.

I probably wouldn't have felt this uneasy iffiness.

Teaching the parser to understand that removal of the prefix "no-" negates an option on top of its existing knowledge that adding it does the same just adds the other side of the same coin, which was curiously missing.

The patch does not forbid adding "no-" to an option that already starts with "no-". This stricter rule would be easy to add, but since that is currently the only way to negate such options, it would break backwards compatibility and thus should be added in a separate patch, if at all.

With the patch, the following guidelines are followed:

	- "no-" means no, for both developers and users.
	- The user doesn't have to to say "no-no-".

The results feels simpler to me.

René
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]