Re: [PATCH v3 4/5] stash: introduce new format create

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

 



On 02/13, Jeff King wrote:
> On Mon, Feb 13, 2017 at 04:57:34PM -0500, Jeff King wrote:
> 
> > Yeah, I think your patch is actually fixing that case. But your search
> > is only part of the story. You found somebody using "-m" explicitly, but
> > what about somebody blindly calling:
> > 
> >   git stash create $*
> > 
> > That's now surprising to somebody who puts "-m" in their message.
> > 
> > > I *think* this regression is acceptable, but I'm happy to introduce
> > > another verb if people think otherwise.
> > 
> > Despite what I wrote above, I'm still inclined to say that this isn't an
> > important regression. I'd be surprised if "stash create" is used
> > independently much at all.
> 
> Just thinking on this more...do we really care about "fixing" the
> interface of "stash create"? This is really just about refactoring what
> underlies the new "push", right?
>
> So we could just do:
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index 6d629fc43..ee37db135 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -711,7 +711,7 @@ clear)
>  	;;
>  create)
>  	shift
> -	create_stash "$@" && echo "$w_commit"
> +	create_stash -m "$*" && echo "$w_commit"
>  	;;
>  store)
>  	shift
> 
> on top of your patch and keep the external interface the same.
> 
> It might be nice to clean up the interface for "create" to match other
> ones, but from this discussion I think it is mostly a historical wart
> for scripting, and we are OK to just leave its slightly-broken interface
> in place forever.

Yeah tbh I don't personally care too much about modernizing the
interface to git stash create.  What you have above makes a lot of
sense to me.

I also just noticed that git stash create -m message is not the worst
regression I introduced when trying to modernize this.  In that case
only the -m would go missing, but that's probably not the end of the
world.  The worse thing to do would be something like
git stash create -u untracked, where the intended message was "-u
untracked", but instead there is no message, but all untracked files
are now included in the stash as well.

In that light what you have above makes even more sense to me.
Thanks!

> I could go either way.
> 
> -Peff

-- 
Thomas



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