From: Jonathan Nieder <jrnieder@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Matthieu Moy <Matthieu.Moy@imag.fr>, git@vger.kernel.org
Subject: Re: [PATCH 1/5 v4] diff: parse detached options like -S foo
Date: Mon, 2 Aug 2010 14:47:24 -0500 [thread overview]
Message-ID: <20100802194724.GC2180@burratino> (raw)
In-Reply-To: <7v62ztnesc.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>> + if (arg[2] != '\0') {
>> + *optarg = arg + strlen("-c");
>
> Just a style thing, but I think "arg + 2" is much easier to read in this
> particular case, as it won't risk tempting readers to go "Huh? What does
> that 'c' mean"?
Yeah, especially after the "if (arg[2] != ...)". Thanks for a sanity
check.
> Do we have an option that can take a zero-length string as its value and
> do something meaningful? I don't think of any offhand ("log -S'' -p" is
> not it---it may be meaningful but it is not useful), but this code would
> start giving "-p" instead of "" to the option in such a case.
-l<num>: "" is not a number.
-S<artifact>:
Could "" come up if a user tries to find a commit
"Adding/removing string:" without typing a string to look
for in gitk? No, gitk protects against that.
How about creating a view with an empty "Search string:"?
No, that is protected against, too.
Currently "git log -S" does not seem to do anything
meaningful, anyway.
-O<file>: "" is not a file name.
next prev parent reply other threads:[~2010-08-02 19:48 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-29 8:20 [PATCH 0/5 v3] log and diff: accept detached forms (--option value) Matthieu Moy
2010-07-29 8:20 ` [PATCH 1/5] diff: parse detached options like -S foo Matthieu Moy
2010-07-29 8:20 ` [PATCH 2/5] diff: split off a function for --stat-* option parsing Matthieu Moy
2010-07-29 8:20 ` [PATCH 3/5] diff: parse detached options --stat-width n, --stat-name-width n Matthieu Moy
2010-07-29 8:20 ` [PATCH 4/5] log: parse detached options like git log --grep foo Matthieu Moy
2010-07-29 22:25 ` Jonathan Nieder
2010-07-30 8:27 ` Matthieu Moy
2010-07-30 8:31 ` [PATCH 1/5 v4] diff: parse detached options like -S foo Matthieu Moy
2010-08-02 16:46 ` Junio C Hamano
2010-08-02 19:43 ` Matthieu Moy
2010-08-02 19:47 ` Jonathan Nieder [this message]
2010-07-30 8:31 ` [PATCH 2/5 v4] diff: split off a function for --stat-* option parsing Matthieu Moy
2010-07-30 8:31 ` [PATCH 3/5 v4] diff: parse detached options --stat-width n, --stat-name-width n Matthieu Moy
2010-08-02 16:56 ` Junio C Hamano
2010-08-02 18:47 ` Matthieu Moy
2010-07-30 8:31 ` [PATCH 4/5 v4] log: parse detached options like git log --grep foo Matthieu Moy
2010-08-02 17:03 ` Junio C Hamano
2010-08-02 18:55 ` Matthieu Moy
2010-08-02 21:28 ` Junio C Hamano
2010-08-03 6:33 ` Matthieu Moy
2010-08-03 17:16 ` Junio C Hamano
2010-08-03 19:52 ` Matthieu Moy
2010-08-04 2:41 ` Miles Bader
2010-07-30 8:31 ` [PATCH 5/5 v4] log: parse detached option for --glob Matthieu Moy
2010-07-29 8:20 ` [PATCH 5/5] " Matthieu Moy
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:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100802194724.GC2180@burratino \
--to=jrnieder@gmail.com \
--cc=Matthieu.Moy@imag.fr \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.