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

Re: [PATCH/RFC 1/3] stash: show less information for stash push -- <pathspec>

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 20, 2017, 19:42 UTC
Message-ID
<xmqqtw6nvip1.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170320184855.x7m2gxwdqdt3lnet@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 36 quoted lines
> On Mon, Mar 20, 2017 at 10:51:16AM -0700, Junio C Hamano wrote:
>
>> > diff --git a/git-stash.sh b/git-stash.sh
>> > index 9c70662cc8..59f055e27b 100755
>> > --- a/git-stash.sh
>> > +++ b/git-stash.sh
>> > @@ -299,10 +299,10 @@ push_stash () {
>> >  	then
>> >  		if test $# != 0
>> >  		then
>> > -			git reset ${GIT_QUIET:+-q} -- "$@"
>> > +			git reset -q -- "$@"
>> >  			git ls-files -z --modified -- "$@" |
>> >  			git checkout-index -z --force --stdin
>> > -			git clean --force ${GIT_QUIET:+-q} -d -- "$@"
>> > +			git clean --force -q -d -- "$@"
>> >  		else
>> >  			git reset --hard ${GIT_QUIET:+-q}
>> >  		fi
>> 
>> Yup, we only said "HEAD is now at ..." in the non-pathspec case (and
>> we of course still do).  We didn't report changes to which paths
>> have been reverted in (or which new paths are removed from) the
>> working tree to the original state (and we of course still don't).
>> 
>> The messages from reset and clean that reports these probably do not
>> need to be shown by default to the users (they can always check with
>> "git stash show" when they are curious or when they want to double
>> check).
>
> I'm not sure if you are arguing here that the non-pathspec case should
> move to an unconditional "-q", too, to suppress the "HEAD is now at"
> message.  But I think that is a good suggestion. It would make the two
> cases consistent, and it is not really adding anything of value (it is
> always just HEAD, and if you do not provide a custom message, the
> short-sha1 and subject are already in the "Saved..." line above).

I wasn't suggesting it (I was just saying that these extra messages are not something we found necessary for consistency with the original codepath when we added the pathspec support). I wasn't even thinking about what the original codepath did, i.e. when the command is run without pathspec.

I too suspect that most of the ${GIT_QUIET:+-q} can just become an unconditional -q as you do.

Previous: Jeff KingNext: Thomas Gummerer
Message 7 of 26 in “[BUG] "git stash -- path" reports wrong unstaged changes”
  1. Jeff KingMar 17, 2017
  2. Thomas GummererMar 18, 2017
  3. Thomas GummererMar 19, 2017
  4. 1/3 stash: show less information for stash push -- <pathspec>Thomas Gummerer, Mar 19, 2017
  5. Junio C HamanoMar 20, 2017
  6. Jeff KingMar 20, 2017
  7. Junio C HamanoMar 20, 2017
  8. Thomas GummererMar 21, 2017
  9. Jeff KingMar 20, 2017
  10. 3/3 stash: pass the pathspec argument to git resetThomas Gummerer, Mar 19, 2017
  11. Jeff KingMar 20, 2017
  12. Thomas GummererMar 21, 2017
  13. Jeff KingMar 21, 2017
  14. 2/3 stash: make push -p -q --no-keep-index quietThomas Gummerer, Mar 19, 2017
  15. Jeff KingMar 20, 2017
  16. 0/3 Re: [BUG] "git stash -- path" reports wrong unstaged changesThomas Gummerer, Mar 21, 2017
  17. 2/3 stash: pass the pathspec argument to git resetThomas Gummerer, Mar 21, 2017
  18. Jeff KingMar 21, 2017
  19. 1/3 stash: don't show internal implementation detailsThomas Gummerer, Mar 21, 2017
  20. Jeff KingMar 21, 2017
  21. Thomas GummererMar 22, 2017
  22. Jeff KingMar 22, 2017
  23. 3/3 stash: keep untracked files intact in stash -kThomas Gummerer, Mar 21, 2017
  24. Jeff KingMar 21, 2017
  25. Thomas GummererMar 22, 2017
  26. Jeff KingMar 20, 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.