git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Pierre Habouzit <madcoder@debian.org>,
	Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [RFC] Re: Convert 'git blame' to parse_options()
Date: Mon, 23 Jun 2008 14:33:58 -0400	[thread overview]
Message-ID: <20080623183358.GA28941@sigill.intra.peff.net> (raw)
In-Reply-To: <alpine.LFD.1.10.0806231114180.2926@woody.linux-foundation.org>

On Mon, Jun 23, 2008 at 11:20:46AM -0700, Linus Torvalds wrote:

> Actually, I guess "--default" does, but if you try to mix that up with (a) 
> a default head that starts with a dash and (b) git-blame, you're doing 
> something pretty odd.

Yes, and I think that is why "in practice" this works with git-blame.

> And yes, "-n" does too, but if you pass it negative numbers you get what 
> you deserve.

It's worse than that. We assume by default that the option has no
argument, so the argument becomes a non-option parameter to the original
command. Try (with current git-blame, but I think your patch doesn't
change this):

  $ git blame -n 1 git.c
  fatal: bad revision '1'

  $ git blame --default HEAD git.c
  fatal: cannot stat path HEAD: No such file or directory

Oops. Now again, as it happens, git-blame seems to ignore "-n" entirely
(though I would have expected it to limit the depth of the blame
traversal), so maybe people shouldn't be using it.

> The point being, we really _do_ have a real-life existing case for 
> PARSE_OPT_CONTINUE_ON_UNKNOWN, which is hard to handle any other way. 
> Currently you can literally do
> 
> 	git blame --since=April -b Makefile
> 
> and while it's a totally made-up example, it's one I've picked to show 
> exactly what does *not* work with my patch that started this whole thread.
> 
> And guess what you need to fix it?
> 
> If you guessed PARSE_OPT_CONTINUE_ON_UNKNOWN, you win a prize. 

I guessed "to convert the revision and diff options to a format that
parse_options understands, so we can add them as appropriate to the
options tables of the various commands." Do I win anything?

-Peff

  reply	other threads:[~2008-06-23 18:34 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-23  5:15 Convert 'git blame' to parse_options() Linus Torvalds
2008-06-23  6:35 ` Junio C Hamano
2008-06-23 12:28   ` Johannes Schindelin
2008-06-23  8:22 ` [RFC] " Pierre Habouzit
2008-06-23 12:26   ` Johannes Schindelin
2008-06-23 15:53     ` Pierre Habouzit
2008-06-23 16:25       ` Johannes Schindelin
2008-06-23 16:25     ` Linus Torvalds
2008-06-23 16:49       ` Jeff King
2008-06-23 17:06         ` Linus Torvalds
2008-06-23 17:15           ` Jeff King
2008-06-23 17:32             ` Linus Torvalds
2008-06-23 18:15               ` Jeff King
2008-06-23 18:36                 ` Linus Torvalds
2008-06-23 18:20               ` Linus Torvalds
2008-06-23 18:33                 ` Jeff King [this message]
2008-06-23 18:47                   ` Linus Torvalds
2008-06-23 19:16                     ` Linus Torvalds
2008-06-23 21:09                       ` Pierre Habouzit
2008-06-23 21:11                         ` [PATCH] parse-opt: have parse_options_{start,end} Pierre Habouzit
2008-06-23 21:11                           ` [PATCH] parse-opt: Export a non NORETURN usage dumper Pierre Habouzit
2008-06-23 21:11                             ` [PATCH] parse-opt: create parse_options_step Pierre Habouzit
2008-06-23 21:11                               ` [PATCH] parse-opt: do not pring errors on unknown options, return -2 intead Pierre Habouzit
2008-06-23 21:11                                 ` [PATCH] parse-opt: fake short strings for callers to believe in Pierre Habouzit
2008-06-23 22:08                                 ` [PATCH] parse-opt: do not pring errors on unknown options, return -2 intead Junio C Hamano
2008-06-23 22:13                                   ` Pierre Habouzit
2008-06-23 21:23                         ` [RFC] Re: Convert 'git blame' to parse_options() Pierre Habouzit
2008-06-23 21:23                         ` Junio C Hamano
2008-06-23 21:28                           ` Pierre Habouzit
2008-06-23 21:26                         ` Linus Torvalds
2008-06-23 21:41                           ` Linus Torvalds
2008-06-23 21:47                           ` Pierre Habouzit
2008-06-23 22:11                           ` Junio C Hamano
2008-06-23 22:24                             ` Pierre Habouzit
2008-06-23 22:36                               ` Pierre Habouzit
2008-06-23 22:38                               ` Junio C Hamano
2008-06-23 23:31                                 ` Pierre Habouzit
2008-06-23 23:40                                   ` Linus Torvalds
2008-06-23 23:51                                   ` Junio C Hamano
2008-06-24  7:50                                     ` Pierre Habouzit
2008-06-24  1:27                                   ` Jeff King
2008-06-23 19:53                     ` Jeff King
2008-06-23 20:04                       ` Pierre Habouzit
2008-06-23 20:12                       ` Linus Torvalds
2008-06-24  5:35                         ` Jeff King
2008-06-24 16:59                           ` Linus Torvalds
2008-06-24 17:13                             ` Johannes Schindelin
2008-06-24 17:34                             ` Jeff King
2008-06-24 17:44                               ` Linus Torvalds
2008-06-24 19:46                                 ` Jeff King
2008-06-24  0:30                 ` Junio C Hamano
2008-06-24  8:24                   ` Pierre Habouzit
2008-06-24 17:05                     ` Linus Torvalds
2008-06-24 19:30                       ` Pierre Habouzit
2008-06-24 19:43                         ` Pierre Habouzit
2008-06-25  6:09                         ` Johannes Sixt
2008-06-23 17:04       ` Johannes Schindelin
2008-06-23 17:21         ` Linus Torvalds
2008-06-23 18:39           ` Johannes Schindelin
2008-06-23 17:26         ` Jeff King
2008-06-23 18:41           ` Johannes Schindelin
2008-06-23 19:24       ` Pierre Habouzit
2008-06-23 16:11   ` Linus Torvalds
2008-06-24  9:12 ` Making parse-opt incremental, reworked series Pierre Habouzit
2008-06-24  9:12   ` [PATCH 1/7] parse-opt: have parse_options_{start,end} Pierre Habouzit
2008-06-24  9:12     ` [PATCH 2/7] parse-opt: Export a non NORETURN usage dumper Pierre Habouzit
2008-06-24  9:12       ` [PATCH 3/7] parse-opt: create parse_options_step Pierre Habouzit
2008-06-24  9:12         ` [PATCH 4/7] parse-opt: do not pring errors on unknown options, return -2 intead Pierre Habouzit
2008-06-24  9:12           ` [PATCH 5/7] parse-opt: fake short strings for callers to believe in Pierre Habouzit
2008-06-24  9:12             ` [PATCH 6/7] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option Pierre Habouzit
2008-06-24  9:12               ` [PATCH 7/7] Migrate git-blame to parse-option partially Pierre Habouzit
2008-06-24 10:03               ` [PATCH 6/7] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option Pierre Habouzit
2008-06-24 17:18               ` Linus Torvalds
2008-06-24 19:27                 ` Pierre Habouzit
2008-06-24 20:55                 ` Pierre Habouzit
2008-06-24 17:20             ` [PATCH 5/7] parse-opt: fake short strings for callers to believe in Linus Torvalds
2008-06-24 19:26               ` Pierre Habouzit
2008-06-25 15:07                 ` Andreas Ericsson
2008-06-24 20:58             ` [REPLACEMENT PATCH] " Pierre Habouzit
2008-06-26  8:35               ` Pierre Habouzit
2008-06-26  8:40                 ` Junio C Hamano
2008-06-26  9:37                   ` Pierre Habouzit

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=20080623183358.GA28941@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=madcoder@debian.org \
    --cc=torvalds@linux-foundation.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 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 NNTP newsgroup(s).