Thread (32 messages) 32 messages, 5 authors, 2017-02-14

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

From: Jeff King <hidden>
Date: 2017-02-13 23:05:41
Subsystem: the rest · Maintainer: Linus Torvalds

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.
quoted
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.

I could go either way.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help