Re: [PATCH v3 0/5] stash: support pathspec argument

From: Jeff King <peff@peff.net>
Date: Feb 13, 2017, 20:09 UTC
Message-ID: <20170213200950.m3bcyp52wd25p737@sigill.intra.peff.net>
In-Reply-To: <vpq7f4uxjmo.fsf@anie.imag.fr>

On Mon, Feb 13, 2017 at 03:14:55PM +0100, Matthieu Moy wrote:

Show 6 quoted lines

> I don't remember doing this intentionally. The goal was really to
> prevent typos like "git stash -p drop" (which would have been
> interpreted as "git stash save -p drop" previously). So, let's consider
> this "only messages starting with - are accepted" as a bug: moving to
> the new "push" command it a nice opportunity to fix it without worrying
> about compatibility. A bit more about this in this old thread:

Yeah, I consider it a bug that we treat "-foo" as a message there. Fortunately I think you really have to try hard to abuse it:

  # doesn't work, because "save" complains about "-foo" as an option
  git stash -foo

  # does save stash with message "-foo"
  git stash -- -foo

  # doesn't work, because _any_ non-option argument is rejected
  git stash -- use --foo option

Show 16 quoted lines

> I think we can safely allow both
> 
>   git stash -p -- <pathspec...>
>   git stash push -p <pathspec...>
> 
> But allowing the same without -- is a bit more dangerous for the reason
> above:
> 
>   git stash -p drop
> 
> would be interpreted as
> 
>   git stash push -p drop
> 
> (i.e. limit the stash to file named "drop"), and this would almost
> certainly not be what the user wanted.

Is it really that dangerous, though? The likely outcome is Git saying "nope, you don't have any changes to the file named drop". Of course the user may have meant something different, but I feel like "-p" is a good indicator that they are interested in making an actual stash.

Think about this continuum of commands:

  1. git stash droop

  2. git stash -p drop

  3. git stash -p -- drop

  4. git stash push -p drop

I think we can all agree that (4) is reasonable and safe. And I suspect we'd all agree that (1) is suspect (you probably meant "drop", not "push droop").

But between (2) and (3), I don't see much difference. The interesting difference between (1) and (2) is the addition of "-p", which tells us that the user expects to save a stash.

Another way of thinking of it is that "-p" as a command name is an alias for "push -p". If you wanted to err on the conservative side, you could literally implement it that way (so "git stash -k -p foo" would not work; you'd have to use "git stash -p -k foo").

Show 8 quoted lines

> > The other question is how we would deal with the -m flag for
> > specifying a stash message.  I think we could special case it, but
> > that would allow users to do things such as git stash -m apply, which
> > would make the interface a bit more error prone.  So I'm leaning
> > towards disallowing that for now.
> 
> We already have "git commit -m <msg> <pathspec...>", so I think stash
> should just do the same thing as commit. Or, did I miss something?

The complexity is that right now, the first-level decision of "which stash sub-command am I running?" doesn't know about any options. So "git stash -m foo" would be rejected in the name of typo prevention, unless that outer decision learns about "-m" as an option.

-Peff