Re: [PATCH] Have a flag to stop the option parsing at the first argument.

From: Wincent Colaiuta <win@wincent.com>
Date: Dec 17, 2007, 12:06 UTC
Message-ID: <3CF3CEA5-72F1-47D1-ADB9-37F5C2E292A8@wincent.com>
In-Reply-To: <Pine.LNX.4.64.0712171149540.9446@racer.site>

El 17/12/2007, a las 12:50, Johannes Schindelin escribió:

Show 54 quoted lines

> On Mon, 17 Dec 2007, Pierre Habouzit wrote:
>
>> On Mon, Dec 17, 2007 at 11:10:00AM +0000, Wincent Colaiuta wrote:
>>> El 17/12/2007, a las 10:50, Pierre Habouzit escribió:
>>>
>>>> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
>>>> ---
>>>>>  // ...
>>>>>  /* 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"} */
>>>>
>>>>  Or even simpler, with the following specifically tailored patch  
>>>> you
>>>>  can directly write:
>>>>
>>>>  argc = parse_options(argc, argv, git_generic_options,
>>>> 	"git [generic-options] <command> [cmd-options]",
>>>> 	PARSE_OPT_STOP_AT_ARG);
>>>>
>>>>  and then {argc, argv} will exactly be the NULL-terminated array
>>>>  starting with the builtin command. Kind of nice :)
>>>
>>> Indeed, nice ideas. I think all this will lead to a nice UI  
>>> improvement
>>> post-1.5.4.
>>>
>>> About the only thing that I think would merit action *prior* to  
>>> 1.5.4 is
>>> marking the "-p" switch to git (synonym for --paginate) as  
>>> deprecated,
>>> see as it clashes with other commands' uses of that switch ("git  
>>> log -p"
>>> for example). Are there any other conflicting specials that a  
>>> currently
>>> parsed in git.c?
>>
>>  You don't need to, and I'd see that as a regression. With my  
>> proposal,
>> there isn't any kind of need that git commands do not clash with git
>> ones. The parse-option mechanism will properly hide options that are
>> masked this way, dscho wrote the patch for that.
>>
>>  git -p log -p ...
>>
>> just makes sense to me. CVS or SVN e.g. (don't hit me !) have the  
>> same
>> kind of "issues", and I never found that weird.

Yes, we know what it does because we know that "git ... log ..." is actually two commands and each one handles one of the -p switches, but it is much easier to present git as a single tool to the newcomer (and I guess I don't need to argue that case here seeing as the decision has already been taken long ago to talk using dashless forms), and it is much easier to explain to a newcomer something like:

git log --paginate -p

Than:

git -p log -p

(Incidentally, who actually uses "git -p log -p"? Doesn't everybody have a pager set-up by default?)

Show 6 quoted lines

>>  In fact I see this the other way around: git status -p that is in  
>> fact
>> the same as git -p status, is the conveniency, git -p status is the
>> canonical form.
>
> I would even go further: "git status -p" looks utterly wrong to me.

This is exactly the kind of insider knowledge that will baffle newcomers. The canonical form is canonical only because it relies on an historical (and continuing) implementation detail, but we shouldn't expect newcomers to have to learn such details in order to know where to stick their command line arguments; they already have enough things to learn about, I think.

But it doesn't really matter. The proposed changes allow old-timers to continue putting their special options between the "git" and the "command". If you don't want to deprecate the -p special because of the confusion it might cause, I think we should at least not give it a very prominent place in the documentation, nor use it any examples.

Cheers, Wincent