git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: "Thomas Gummerer" <t.gummerer@gmail.com>,
	git@vger.kernel.org, "Stephan Beyer" <s-beyer@gmx.net>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Marc Strapetz" <marc.strapetz@syntevo.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Øyvind A . Holm" <sunny@sunbase.org>,
	"Jakub Narębski" <jnareb@gmail.com>
Subject: Re: [PATCH v3 0/5] stash: support pathspec argument
Date: Mon, 13 Feb 2017 15:09:50 -0500	[thread overview]
Message-ID: <20170213200950.m3bcyp52wd25p737@sigill.intra.peff.net> (raw)
In-Reply-To: <vpq7f4uxjmo.fsf@anie.imag.fr>

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

  parent reply	other threads:[~2017-02-13 20:09 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-21 20:08 [PATCH 0/3] stash: support filename argument Thomas Gummerer
2017-01-21 20:08 ` [PATCH 1/3] Documentation/stash: remove mention of git reset --hard Thomas Gummerer
2017-01-22  1:27   ` Øyvind A. Holm
2017-01-24 19:51   ` Jakub Narębski
2017-01-24 20:14   ` Jeff King
2017-01-25 13:02     ` Jakub Narębski
2017-01-25 21:20       ` Junio C Hamano
2017-01-25 21:20     ` Junio C Hamano
2017-01-28 19:30       ` Thomas Gummerer
2017-01-28 23:54         ` Jeff King
2017-01-21 20:08 ` [PATCH 2/3] stash: introduce push verb Thomas Gummerer
2017-01-23 18:27   ` Junio C Hamano
2017-01-29 12:39     ` Thomas Gummerer
2017-01-21 20:08 ` [PATCH 3/3] stash: support filename argument Thomas Gummerer
2017-01-23 18:50   ` Junio C Hamano
2017-01-29 13:29     ` Thomas Gummerer
2017-01-24 10:56 ` [PATCH 0/3] " Johannes Schindelin
2017-01-29 20:16 ` [PATCH v2 0/4] stash: create " Thomas Gummerer
2017-01-29 20:16   ` [PATCH v2 1/4] Documentation/stash: remove mention of git reset --hard Thomas Gummerer
2017-01-30 21:13     ` Junio C Hamano
2017-02-05 12:13       ` Thomas Gummerer
2017-01-29 20:16   ` [PATCH v2 2/4] stash: introduce push verb Thomas Gummerer
2017-01-30 21:12     ` Junio C Hamano
     [not found]     ` <xmqqy3xsux4g.fsf@gitster.mtv.corp.google.com>
2017-02-04 12:19       ` Thomas Gummerer
2017-01-29 20:16   ` [PATCH v2 3/4] introduce new format for git stash create Thomas Gummerer
2017-01-30 21:10     ` Junio C Hamano
     [not found]     ` <xmqqtw8guwfm.fsf@gitster.mtv.corp.google.com>
2017-02-04 13:18       ` Thomas Gummerer
2017-01-29 20:16   ` [PATCH v2 4/4] stash: support filename argument Thomas Gummerer
2017-01-30 21:11     ` Junio C Hamano
2017-02-05 11:02       ` Thomas Gummerer
2017-02-05 20:26   ` [PATCH v3 0/5] stash: support pathspec argument Thomas Gummerer
2017-02-05 20:26     ` [PATCH v3 1/5] Documentation/stash: remove mention of git reset --hard Thomas Gummerer
2017-02-06 15:22       ` Jeff King
2017-02-05 20:26     ` [PATCH v3 2/5] stash: introduce push verb Thomas Gummerer
2017-02-06 15:46       ` Jeff King
2017-02-11 13:33         ` Thomas Gummerer
     [not found]       ` <vpqlgtaz09q.fsf@anie.imag.fr>
2017-02-13 22:16         ` Thomas Gummerer
2017-02-05 20:26     ` [PATCH v3 3/5] stash: add test for the create command line arguments Thomas Gummerer
2017-02-06 15:50       ` Jeff King
2017-02-11 13:55         ` Thomas Gummerer
2017-02-05 20:26     ` [PATCH v3 4/5] stash: introduce new format create Thomas Gummerer
2017-02-06 15:56       ` Jeff King
2017-02-11 14:51         ` Thomas Gummerer
2017-02-13 21:57           ` Jeff King
2017-02-13 23:05             ` Jeff King
2017-02-14 21:30               ` Thomas Gummerer
2017-02-05 20:26     ` [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec Thomas Gummerer
     [not found]       ` <xmqqmvdz3ied.fsf@gitster.mtv.corp.google.com>
2017-02-06 15:20         ` Jeff King
2017-02-11 16:50         ` Thomas Gummerer
2017-02-06 16:14     ` [PATCH v3 0/5] stash: support pathspec argument Jeff King
2017-02-12 19:30       ` Thomas Gummerer
     [not found]         ` <vpq7f4uxjmo.fsf@anie.imag.fr>
2017-02-13 20:09           ` Jeff King [this message]
     [not found]             ` <vpqo9y5lqos.fsf@anie.imag.fr>
2017-02-13 21:45               ` Jeff King
2017-02-13 22:33                 ` Thomas Gummerer
     [not found]                   ` <xmqqwpctabvw.fsf@gitster.mtv.corp.google.com>
2017-02-14  0:27                     ` Jeff King
     [not found]                       ` <xmqqpoila9rt.fsf@gitster.mtv.corp.google.com>
2017-02-14  0:37                         ` Jeff King
     [not found]                   ` <vpq60kdjl63.fsf@anie.imag.fr>
2017-02-14 21:36                     ` Thomas Gummerer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170213200950.m3bcyp52wd25p737@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@gmail.com \
    --cc=marc.strapetz@syntevo.com \
    --cc=s-beyer@gmx.net \
    --cc=sunny@sunbase.org \
    --cc=t.gummerer@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).