Hi Thomas,
On Mon, 26 Mar 2018, Thomas Gummerer wrote:
> On 03/26, Johannes Schindelin wrote:
> >
> > On Sun, 25 Mar 2018, Eric Sunshine wrote:
> >
> > > On Sat, Mar 24, 2018 at 2:23 PM, Paul-Sebastian Ungureanu
> > > <ungureanupaulsebastian@xxxxxxxxx> wrote:
> > > > Currently, because git stash is not fully converted to C, I
> > > > introduced a new helper that will hold the converted commands.
> > > > ---
> > > > diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> > > > @@ -0,0 +1,52 @@
> > > > +int cmd_stash__helper(int argc, const char **argv, const char *prefix)
> > > > +{
> > > > + int cmdmode = 0;
> > > > +
> > > > + struct option options[] = {
> > > > + OPT_CMDMODE(0, "list", &cmdmode,
> > > > + N_("list stash entries"), LIST_STASH),
> > > > + OPT_END()
> > > > + };
> > >
> > > Is the intention that once git-stash--helper implements all 'stash'
> > > functionality, you will simply rename git-stash--helper to git-stash?
> > > If so, then I'd think that you'd want it to accept subcommand
> > > arguments as bare words ("apply", "drop") in order to be consistent
> > > with the existing git-stash command set, not in dashed form
> >
> > Why not start with cmdmode, and then add a single patch that *also*
> > accepts argv[1] as a bare-word cmdmode?
>
> I don't think we should accept the dashed form of the commands for
> 'git stash'. The main reason being that we also have 'git stash'
> without any arguments, which acts as 'git stash push'. So if we would
> ever come up with an argument to 'git stash push', that matches one of
> the current verbs, or if we come up with a new verb that matches one
> of the options to 'git stash push', that would not work.
>
> In that case we could obviously go for a different word, but I think
> the rules when 'git stash' is going to be 'git stash push' and when it
> is not are already complicated enough, and allowing the verbs as
> dashed options would only make the interface more complicated.
>
> > This could even potentially be a patch to parse-options.[ch] that
> > introduces, say, PARSE_OPT_ALLOW_BARE_CMDMODE.
>
> Now if we'd take that one step further and make it
> PARSE_OPT_BARE_CMDMODE, which would only allow the non-dashed options,
> I think we could use that in other places in git as well (for example
> in 'git worktree').
But the valid options change with the subcommands, too. So my idea, even
with your adjustment to disallow dashed cmdmode, simply does not work.
Thanks for setting my head straight,
Dscho
