git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Pierre Habouzit <madcoder@debian.org>
Cc: git@vger.kernel.org, Lars Hjemli <hjemli@gmail.com>
Subject: Re: [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag.
Date: Tue, 08 Jul 2008 18:15:43 -0700	[thread overview]
Message-ID: <7vvdzfoo1s.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: 20080708103408.GC19202@artemis.madism.org

Pierre Habouzit <madcoder@debian.org> writes:

> If you set this for a given flag, and the flag appears without a value on
> the command line, then the `defval' is used to fake a new argument.
>
> Note that this flag is meaningless in presence of OPTARG or NOARG flags.
> (in the current implementation it will be ignored, but don't rely on it).
>
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>
>     >   (3) inspired from (1) and (2), have a flag for options that says
>     >       "I do take an argument, but if I'm the last option on the
>     >       command line, please fake this argument for me.
>     >
>     > I really like (3) more FWIW as it doesn't generate ambiguous
>     > parsers like (2) would, and it's not horrible like (1). And cherry
>     > on top it's pretty trivial to implement I think.

Yeah, I do not particularly want a major rewrite that only introduces
possible ambiguity to the option parser (even though arguably it would add
to the usability very much, just like we accept revs and paths when
unambiguous), so I think this is a reasonable compromise.

It feels more like LASTARG_DEFAULT but that is bikeshedding ;-)

But I see one thinko (fix below) and another issue I am not sure what the
best fix would be.

---
diff --git a/parse-options.c b/parse-options.c
index b6735a5..cba20d7 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -26,11 +26,11 @@ static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
 	if (p->opt) {
 		*arg = p->opt;
 		p->opt = NULL;
+	} else if (p->argc == 1 && (opt->flags & PARSE_OPT_FAKELASTARG)) {
+		*arg = (const char *)opt->defval;
 	} else if (p->argc) {
 		p->argc--;
 		*arg = *++p->argv;
-	} else if (opt->flags & PARSE_OPT_FAKELASTARG) {
-		*arg = (const char *)opt->defval;
 	} else
 		return opterror(opt, "requires a value", flags);
 	return 0;

  reply	other threads:[~2008-07-09  1:16 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-08  6:49 What should "git branch --merged master" do? Junio C Hamano
2008-07-08 10:14 ` Pierre Habouzit
2008-07-08 10:34   ` [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag Pierre Habouzit
2008-07-09  1:15     ` Junio C Hamano [this message]
2008-07-09  1:17       ` [PATCH 1/2] branch --contains: default to HEAD Junio C Hamano
2008-07-09  1:22       ` [PATCH 2/2] branch --merged/--not-merged: allow specifying arbitrary commit Junio C Hamano
2008-07-09  7:45         ` Pierre Habouzit
2008-07-09  9:13           ` Junio C Hamano
2008-07-09  7:47       ` [PATCH] parse-options: add PARSE_OPT_FAKELASTARG flag 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=7vvdzfoo1s.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=hjemli@gmail.com \
    --cc=madcoder@debian.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).