From: Pierre Habouzit <madcoder@debian•org>
To: Wincent Colaiuta <win@wincent•com>
Cc: Junio C Hamano <gitster@pobox•com>, git@vger•kernel.org
Subject: Re: git.c option parsing
Date: Mon, 17 Dec 2007 10:20:13 +0100 [thread overview]
Message-ID: <20071217092013.GD7453@artemis.madism.org> (raw)
In-Reply-To: <25018761-80E8-410C-BB18-DD799F58308A@wincent.com>
[-- Attachment #1: Type: text/plain, Size: 2703 bytes --]
On Mon, Dec 17, 2007 at 09:01:56AM +0000, Wincent Colaiuta wrote:
> El 17/12/2007, a las 9:48, Junio C Hamano escribió:
>
> >Wincent Colaiuta <win@wincent•com> writes:
> >
> >>Of course, the above plan will only work for builtins, not for
> >>scripts. An additional step would be needed to enable scripts to
> >>handle these options; perhaps teaching "git rev-parse" something...
> >
> >As long as special options stay special and we make a rule not to allow
> >any subcommand to assign its own meaning to them, the git wrapper can
> >lookahead and reorder, when seeing a command line:
> >
> > git scripted-command --special
> >
> >into
> >
> > git --special scripted-command
> >
> >And that approach would work well for built-ins as well, I would
> >imagine.
>
> Yes, and it would be simpler to implement also. The only downside is that
> without all the other proposed changes things like "git-dashed --special"
> wouldn't work; only teaching the builtins to actually handle the special
> options would work in that case. And in the interests of consistency I
> think it's pretty important that "git subcommand --special" and
> "git-subcommand --special" both work the same as the user would
> (reasonably) expect them to...
There is a simple way to do that, that wouldn't conflict with the git
grep -e one, that would be to define an array of "Special" flags, in a
macro, and have every builtin using parseopt adding that macro at the
end.
Then in git.c, you would have to scan for the command name when called
through `git --opt1 --opt2 foo`, and pass it to parseopt with as argc
the position of `foo` in the argument array. Parseopt will trust you for
it, and if it returns a non zero count, you have to barf, then you just
need to work on the rest of the array. That would mean in pseudo code
that this would work like:
// ...
/* when in `git --opt1 --opt2 foo -a -b -c` mode: */
int cmd_pos = git_find_builtin_command_name(argc, argv);
int count = parse_options(cmd_pos, argv, git_generic_options,
"git [special-options] cmd [options]", 0);
if (count)
die("unknown git command: `%s`", argv[0]);
argv += cmd_pos;
argc -= cmd_pos;
/* here we simulate an argv of {"foo", "-a", "-b", "-c"} */
Of course this only works on builtins that do support parseopt other
ones will need the massage you describe. For them the sole thing to
change would be to replace the OPT_END() with a GIT_SPECIAL_OPTS() or
sth alike.
--
·O· Pierre Habouzit
··O madcoder@debian•org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
next prev parent reply other threads:[~2007-12-17 9:20 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-12-14 11:23 [PATCH 0/3] diff topic tweaks Wincent Colaiuta
2007-12-14 11:23 ` [PATCH 1/3] Revert changes and extend diff option documentation Wincent Colaiuta
2007-12-14 11:23 ` [PATCH 2/3] Use shorter error messages for whitespace problems Wincent Colaiuta
2007-12-14 11:23 ` [PATCH 3/3] Test interaction between diff --check and --exit-code Wincent Colaiuta
2007-12-16 19:43 ` [PATCH 1/3] Revert changes and extend diff option documentation Junio C Hamano
2007-12-17 8:27 ` git.c option parsing (was: [PATCH 1/3] Revert changes and extend diff option documentation) Wincent Colaiuta
2007-12-17 8:48 ` git.c option parsing Junio C Hamano
2007-12-17 9:01 ` Wincent Colaiuta
2007-12-17 9:20 ` Pierre Habouzit [this message]
2007-12-17 9:26 ` Pierre Habouzit
2007-12-17 9:50 ` [PATCH] Have a flag to stop the option parsing at the first argument Pierre Habouzit
2007-12-17 10:34 ` Johannes Schindelin
2007-12-17 11:10 ` Wincent Colaiuta
2007-12-17 11:47 ` Pierre Habouzit
2007-12-17 11:50 ` Johannes Schindelin
2007-12-17 12:06 ` Wincent Colaiuta
2007-12-17 12:26 ` Johannes Schindelin
2007-12-17 12:40 ` Wincent Colaiuta
2007-12-17 12:55 ` Johannes Schindelin
2007-12-17 13:12 ` Wincent Colaiuta
2007-12-17 13:30 ` Johannes Schindelin
2007-12-17 13:57 ` Wincent Colaiuta
2007-12-17 15:13 ` Johannes Sixt
2007-12-17 16:29 ` 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=20071217092013.GD7453@artemis.madism.org \
--to=madcoder@debian$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=win@wincent$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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