All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: Re: [PATCH/RFC 1/3] stash: show less information for stash push -- <pathspec>
Date: Tue, 21 Mar 2017 20:48:42 +0000	[thread overview]
Message-ID: <20170321204842.GD27158@hank> (raw)
In-Reply-To: <xmqqtw6nvip1.fsf@gitster.mtv.corp.google.com>

On 03/20, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > 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.

Thanks both, I do agree that passing -q unconditionally is probably
the right thing to do.  Will do that, and also pass -q unconditionally
to the git reset I addressed in 2/3 here in the re-roll.

  reply	other threads:[~2017-03-21 20:48 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-17 14:50 [BUG] "git stash -- path" reports wrong unstaged changes Jeff King
2017-03-18 18:36 ` Thomas Gummerer
2017-03-19 20:23   ` Thomas Gummerer
2017-03-19 20:23     ` [PATCH/RFC 1/3] stash: show less information for stash push -- <pathspec> Thomas Gummerer
2017-03-20 17:51       ` Junio C Hamano
2017-03-20 18:48         ` Jeff King
2017-03-20 19:42           ` Junio C Hamano
2017-03-21 20:48             ` Thomas Gummerer [this message]
2017-03-20 18:42       ` Jeff King
2017-03-19 20:23     ` [PATCH/RFC 2/3] stash: make push -p -q --no-keep-index quiet Thomas Gummerer
2017-03-20 18:55       ` Jeff King
2017-03-19 20:23     ` [PATCH/RFC 3/3] stash: pass the pathspec argument to git reset Thomas Gummerer
2017-03-20 19:08       ` Jeff King
2017-03-21 21:14         ` Thomas Gummerer
2017-03-21 22:12           ` Jeff King
2017-03-21 22:12     ` [PATCH v2 0/3] Re: [BUG] "git stash -- path" reports wrong unstaged changes Thomas Gummerer
2017-03-21 22:12       ` [PATCH v2 1/3] stash: don't show internal implementation details Thomas Gummerer
2017-03-21 22:14         ` Jeff King
2017-03-22 21:33           ` Thomas Gummerer
2017-03-22 21:41             ` Jeff King
2017-03-21 22:12       ` [PATCH v2 2/3] stash: pass the pathspec argument to git reset Thomas Gummerer
2017-03-21 22:19         ` Jeff King
2017-03-21 22:12       ` [PATCH v2 3/3] stash: keep untracked files intact in stash -k Thomas Gummerer
2017-03-21 22:38         ` Jeff King
2017-03-22 21:46           ` Thomas Gummerer
2017-03-20 18:41   ` [BUG] "git stash -- path" reports wrong unstaged changes Jeff King

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

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

  git send-email \
    --in-reply-to=20170321204842.GD27158@hank \
    --to=t.gummerer@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.