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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> 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

> 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").

> > 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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]