mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <>
To: Christian Couder <>
Cc: git <>, Christian Couder <>
Subject: Re: [PATCH 1/3] git-compat-util: introduce skip_to_opt_val()
Date: Mon, 04 Dec 2017 05:35:07 -0800	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <> (Christian Couder's message of "Mon, 4 Dec 2017 08:59:12 +0100")

Christian Couder <> writes:

> In the few cases where do_something() accepts NULL and does something
> different with it, the former can be changed to:
>         if (skip_to_optional_val(arg, "--key", &arg, NULL) /* the last
> argument is the default value */
>                 do_something(arg);

That's the thing.  If do_something() that takes "" and NULL and
behaves differently is rare, that indicates that the existing code
may not be committed to treat "--key" and "--key=''" the same in the
first place, and I am not 100% convinced that I want to see us
committed to force that design throughout the system by introducing
a helper that hardcodes the equivalence and encourages to use it.

Imagine a command that takes "--do-something" option and does that
"something" unconditionally.  We may later extend it to take an
optional argument, i.e. "--do-something=c1,c2,...", to tell the
command to do that "something" under some but not all conditions.
The values c1,c2 would tell that we want that something done only
under either conditions c1 or c2 holds true.

It would be natural to expect that "--do-something=" to do that
"something" under no condition (i.e. as if no such option was
given); that would help scripts that accumulate the set of
conditions in a variable and say "--do-something=$when", by making
it a no-op when the variable $when turns out to be an empty string.
"--do-something" without "=" would not want to mean the same thing.

The above observation makes me suspect that it depends on the "key"
what "--key=$value" we want to be equivalent to "--key".  In the
"--do-something" case, we do not want to pretend as if we got an
empty string; instead we'd pretend as if we got "always" or
something like that.

And your "default" would work well for this "default is tied to what
the key is" paradigm, i.e.

	skip_to_optional_arg(arg, "--do-something", &arg, "always")

would make us treat "--do-something" and "--do-something=always" the
same way.

If it turns out that the default arg almost always is an empty
string, I do not mind 

	#define skip_to_opt_arg(s,k,v) skip_to_optional_arg(s,k,v,"")

of course.

      reply	other threads:[~2017-12-04 13:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-03 17:04 [PATCH 1/3] git-compat-util: introduce skip_to_opt_val() Christian Couder
2017-12-03 17:04 ` [PATCH 2/3] index-pack: use skip_to_opt_val() Christian Couder
2017-12-03 17:04 ` [PATCH 3/3] diff: " Christian Couder
2017-12-07  0:16   ` Jacob Keller
2017-12-07  0:18     ` Jacob Keller
2017-12-07  6:26       ` Christian Couder
2017-12-03 18:45 ` [PATCH 1/3] git-compat-util: introduce skip_to_opt_val() Junio C Hamano
2017-12-03 20:34   ` Christian Couder
2017-12-03 22:48     ` Junio C Hamano
2017-12-04  7:59       ` Christian Couder
2017-12-04 13:35         ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).