git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Thomas Gummerer <t.gummerer@gmail.com>
Date
Feb 13, 2017, 22:33 UTC
Message-ID
<20170213223346.GD652@hank>
In-Reply-To
<20170213214521.pkjesijdlus36tnp@sigill.intra.peff.net>
On 02/13, Jeff King wrote:
Show 57 quoted lines
> On Mon, Feb 13, 2017 at 10:35:31PM +0100, Matthieu Moy wrote:
> 
> > > 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.
> > 
> > Indeed -p is not the best example. In the old thread, I used -q which is
> > much more problematic:
> > 
> >   git stash -q drop => interpreted as: git stash push -q drop
> >   git stash drop -q => drop with option -q
> 
> Yeah, I'd agree with that. I wouldn't propose to loosen it entirely, but
> rather to treat "-p" specially.
> 
> > It's not really "dangerous" at least in this case, since we misinterpret
> > a destructive command for a less destructive one, but it is rather
> > confusing that changing the order between command and options change the
> > behavior.
> > 
> > I actually find it a reasonable expectation to allow swapping commands
> > and options, some programs other than git allow it.
> 
> I think we may have already crossed that bridge with "git -p stash".
> 
> Not to mention that the ordering already _is_ relevant (we disallow one
> order but not the other). If we really wanted to allow swapping, it
> would mean making:
> 
>   git stash -p drop
> 
> the same as:
> 
>   git stash drop -p
> 
> I actually find _that_ more confusing. It would perhaps make more sense
> with something like "-q", which is more of a "global" option than a
> command-specific one. But I think we'd want to whitelist such global
> options (and "-p" would not be on that list).
>
> > > 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.
> > 
> > Ah, OK. But that's not really hard to implement: when going through the
> > option list looking for non-option, shift one more time when finding -m.
> 
> No, it's not hard conceptually. It just means implementing the
> option-parsing policy in two places. That's not too bad now, but if we
> started using rev-parse's options helper, then I think you have corner
> cases like "git stash -km foo".
> 
> My "-p" suggestion suffers from a similar problem if you treat it as
> "you can omit the 'push' if you say "-p", rather than "if -p is the
> first option, it is a synonym for 'push -p'".

I'm almost convinced of special casing "-p". (Maybe I'm easy to convince as well, because it would be convenient ;) ) However it's a bit weird that now "git stash -p file" would work, but "git stash -m message" wouldn't. Maybe we should do it the other way around, and only special case "-q", and see if there is an non option argument after that? From a glance at the options that's the only one where "git stash -<option> <verb>" could make sense to the user.

Previous: Jeff KingNext: Thomas Gummerer
Message 53 of 57 in “stash: support filename argument”
  1. 0/3 stash: support filename argumentThomas Gummerer, Jan 21, 2017
  2. 1/3 Documentation/stash: remove mention of git reset --hardThomas Gummerer, Jan 21, 2017
  3. Øyvind A. HolmJan 22, 2017
  4. Jakub NarębskiJan 24, 2017
  5. Jeff KingJan 24, 2017
  6. Jakub NarębskiJan 25, 2017
  7. Junio C HamanoJan 25, 2017
  8. Junio C HamanoJan 25, 2017
  9. Thomas GummererJan 28, 2017
  10. Jeff KingJan 28, 2017
  11. 2/3 stash: introduce push verbThomas Gummerer, Jan 21, 2017
  12. Junio C HamanoJan 23, 2017
  13. Thomas GummererJan 29, 2017
  14. 3/3 stash: support filename argumentThomas Gummerer, Jan 21, 2017
  15. Junio C HamanoJan 23, 2017
  16. Thomas GummererJan 29, 2017
  17. Johannes SchindelinJan 24, 2017
  18. 0/4 stash: create filename argumentThomas Gummerer, Jan 29, 2017
  19. 3/4 introduce new format for git stash createThomas Gummerer, Jan 29, 2017
  20. Junio C HamanoJan 30, 2017
  21. 2/4 stash: introduce push verbThomas Gummerer, Jan 29, 2017
  22. Junio C HamanoJan 30, 2017
  23. 1/4 Documentation/stash: remove mention of git reset --hardThomas Gummerer, Jan 29, 2017
  24. Junio C HamanoJan 30, 2017
  25. Thomas GummererFeb 5, 2017
  26. 4/4 stash: support filename argumentThomas Gummerer, Jan 29, 2017
  27. Junio C HamanoJan 30, 2017
  28. Thomas GummererFeb 5, 2017
  29. 0/5 stash: support pathspec argumentThomas Gummerer, Feb 5, 2017
  30. 2/5 stash: introduce push verbThomas Gummerer, Feb 5, 2017
  31. Jeff KingFeb 6, 2017
  32. Thomas GummererFeb 11, 2017
  33. 4/5 stash: introduce new format createThomas Gummerer, Feb 5, 2017
  34. Jeff KingFeb 6, 2017
  35. Thomas GummererFeb 11, 2017
  36. Jeff KingFeb 13, 2017
  37. Jeff KingFeb 13, 2017
  38. Thomas GummererFeb 14, 2017
  39. 3/5 stash: add test for the create command line argumentsThomas Gummerer, Feb 5, 2017
  40. Jeff KingFeb 6, 2017
  41. Thomas GummererFeb 11, 2017
  42. 1/5 Documentation/stash: remove mention of git reset --hardThomas Gummerer, Feb 5, 2017
  43. Jeff KingFeb 6, 2017
  44. 5/5 stash: teach 'push' (and 'create') to honor pathspecThomas Gummerer, Feb 5, 2017
  45. Jeff KingFeb 6, 2017
  46. Thomas GummererFeb 12, 2017
  47. Thomas GummererFeb 4, 2017
  48. Thomas GummererFeb 4, 2017
  49. Jeff KingFeb 6, 2017
  50. Thomas GummererFeb 11, 2017
  51. Jeff KingFeb 13, 2017
  52. Jeff KingFeb 13, 2017
  53. Thomas GummererFeb 13, 2017
  54. Thomas GummererFeb 13, 2017
  55. Jeff KingFeb 14, 2017
  56. Jeff KingFeb 14, 2017
  57. Thomas GummererFeb 14, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.