From: Thomas Rast <tr@thomasrast.ch>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jonathan Nieder <jrnieder@gmail.com>,
Paul Mackerras <paulus@samba.org>,
Jens Lehmann <Jens.Lehmann@web.de>
Subject: Re: [PATCH v2 6/7] Documentation: put blame/log -L in sticked form
Date: Wed, 30 Oct 2013 19:59:50 +0100 [thread overview]
Message-ID: <874n7ywpnd.fsf@thomasrast.ch> (raw)
In-Reply-To: <xmqqiowed6t3.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Wed, 30 Oct 2013 10:09:28 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> Thomas Rast <tr@thomasrast.ch> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> I agree that this patch may reduce confusion locally, but if we were
>>> to go in this direction, we should be consistent and enforce "stuck"
>>> form everywhere,
>>
>> Hmm. Do you want to go there?
>
> Absolutely not ;-)
>
> But that unpleasant place would be the logical conclusion where this
> patch leads us to, I would have to say. I was hoping that there is
> an alternative solution to avoid that.
>
> For example, gitk's parseviewargs is very well aware of the options
> it supports, and it goes through the argument list one by one,
> acting on what option it is looking at. Couldn't it be extended to
> handle options with stuck and unstuck form? After all, it has to
> know that "-L" and "-S" are supported options; it wouldn't be too
> much to ask for the parser to also know that "-L" eats the next
> token (i.e. pass the pair <"-L", next token> intact as two separate
> args to the underlying "log") while it can pass "-L?*" as is, no?
It's not quite that easy because gitk does two-stage processing, and the
big switch you are discussing here is only the second one. The first
one is git-rev-parse, and while it happens to know about '-n 1', it does
not recognize any other unstuck option arguments. (I haven't stared too
long, but I think git-rev-parse is important to distinguish revisions
from paths.)
I actually burned some train time today looking into this, and the
situation is much worse than I thought. There is absolutely no
consistency in any dimension:
a) many commands use parse_options internally, where mandatory args can
be stuck or unstuck, but optional args must be stuck
a1) git branch --{contains,merged,no-merged} take a mandatory arg,
except if they are last on the command line, in which case the
option reverts to the default (HEAD). Effectively this means the
argument is half-optional but the spelling seen in the wild is
usually unstuck.
a2) git-rev-parse (at least) still handrolls its parsing, so no
--default=HEAD
a3) git-commit-tree does not understand any of its short options in
stuck form (!)
b) the perl scripts mostly seem to be using Getopt::Long which handles
things similarly, though I can't quote chapter&verse
b1) just to prove a point: git-add--interactive. I'm sure there's a
user-facing exception somewhere too...
c) shell scripts mostly go through git-sh-setup, using parseopt
internally
c1) git-filter-branch
d) gitk doesn't do *un*stuck as explained above
On top of that, documentation is a wild mash of styles, sometimes even
in the same manpage. For example, git-describe(1) tells the poor user
about --candidates=<n> and four paragraphs further down about --match
<pattern>.
So my short-term plan just became: document instead of fix; clean up
manpages towards the stuck form for long options; have gitk only parse
-Lstuck.
Medium term we can move gitk to a different option parser, resolving at
least that inconsistency.
Longer term we can see about moving some more of the remaining craziness
towards parseopt, getting consistency for free.
--
Thomas Rast
tr@thomasrast.ch
next prev parent reply other threads:[~2013-10-30 19:00 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-09 19:44 [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 1/4] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 2/4] gitk: split out diff part in $commitinfo Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 3/4] gitk: support showing the gathered inline diffs Thomas Rast
2013-06-09 19:44 ` [PATCH gitk 4/4] gitk: recognize -L option Thomas Rast
2013-07-23 15:19 ` [PATCH gitk 0/4] gitk support for git log -L Thomas Rast
2013-07-29 19:37 ` Thomas Rast
2013-07-29 20:07 ` Jens Lehmann
2013-07-31 13:17 ` Thomas Rast
2013-08-18 11:54 ` Paul Mackerras
2013-08-19 8:21 ` Thomas Rast
2013-08-19 17:30 ` Junio C Hamano
2013-10-13 6:31 ` Thomas Rast
2013-10-14 5:25 ` Jonathan Nieder
2013-10-20 16:57 ` [PATCH] Documentation: revamp gitk(1) Thomas Rast
2013-10-29 7:20 ` [PATCH v2 0/7] gitk -L Thomas Rast
2013-10-29 7:20 ` [PATCH v2 1/7] gitk: support -G option from the command line Thomas Rast
2013-10-30 0:52 ` Junio C Hamano
2013-10-30 6:30 ` Thomas Rast
2013-10-30 16:42 ` Junio C Hamano
2013-10-29 7:20 ` [PATCH v2 2/7] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
2013-10-29 7:20 ` [PATCH v2 3/7] gitk: split out diff part in $commitinfo Thomas Rast
2013-10-29 7:20 ` [PATCH v2 4/7] gitk: support showing the gathered inline diffs Thomas Rast
2013-10-29 7:20 ` [PATCH v2 5/7] gitk: recognize -L option Thomas Rast
2013-10-29 7:20 ` [PATCH v2 6/7] Documentation: put blame/log -L in sticked form Thomas Rast
2013-10-30 1:11 ` Junio C Hamano
2013-10-30 6:29 ` Thomas Rast
2013-10-30 17:09 ` Junio C Hamano
2013-10-30 18:59 ` Thomas Rast [this message]
2013-10-30 19:37 ` Junio C Hamano
2013-11-16 17:37 ` [PATCH v3 gitk 0/5] gitk -L Thomas Rast
2013-11-16 17:37 ` [PATCH v3 gitk 1/5] gitk: support -G option from the command line Thomas Rast
2013-11-16 17:37 ` [PATCH v3 gitk 2/5] gitk: refactor per-line part of getblobdiffline and its support Thomas Rast
2013-11-16 17:37 ` [PATCH v3 gitk 3/5] gitk: split out diff part in $commitinfo Thomas Rast
2013-11-16 17:37 ` [PATCH v3 gitk 4/5] gitk: support showing the gathered inline diffs Thomas Rast
2013-11-16 17:37 ` [PATCH v3 gitk 5/5] gitk: recognize -L option Thomas Rast
2013-12-01 22:25 ` [PATCH v3 gitk 0/5] gitk -L Paul Mackerras
2013-11-16 17:37 ` [PATCH v3 0/3] Documentation: stuck arguments and gitk log -L Thomas Rast
2013-11-16 17:37 ` [PATCH v3 1/3] commit-tree: use prefixcmp instead of memcmp(..., N) Thomas Rast
2013-11-16 17:37 ` [PATCH v3 2/3] Documentation: convert to --option=arg form where possible Thomas Rast
2013-11-16 17:37 ` [PATCH v3 3/3] Documentation/gitk: document -L option Thomas Rast
2013-10-29 7:20 ` [PATCH v2 7/7] " Thomas Rast
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=874n7ywpnd.fsf@thomasrast.ch \
--to=tr@thomasrast.ch \
--cc=Jens.Lehmann@web.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=paulus@samba.org \
/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.