threads / patch / 47301

v2Teach stash to parse -m/--message like commit does

Subject: [PATCH v2] Teach stash to parse -m/--message like commit does

## tl;dr

3 messages between Nov 22, 2017 and Nov 24, 2017. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Phil Hord· Nov 22, 2017, 21:20 UTC · lore

`git stash push -m foo` uses "foo" as the message for the stash. But `git stash push -m"foo"` does not parse successfully. Similarly `git stash push --message="My stash message"` also fails. The stash documentation doesn't suggest this syntax should work, but gitcli does and my fingers have learned this pattern long ago for `commit`.

Teach `git stash` and `git store` to parse -mFoo and --message=Foo the same as `git commit` would do. Even though it's an internal function, add similar support to create_stash() for consistency.

Reviewd-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phil Hord <phil.hord@gmail.com>
---

Added tests for 'stash push' and 'stash store'. Added a note that create_stash is included but unnecessary.

 git-stash.sh     | 18 +++++++++++
 t/t3903-stash.sh | 93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

Show changes to 2 files +111 −0

git-stash.sh, t/t3903-stash.sh

diff --git a/git-stash.sh b/git-stash.sh
index 4b7495144..1114005ce 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -76,6 +76,12 @@ create_stash () {
 			shift
 			stash_msg=${1?"BUG: create_stash () -m requires an argument"}
 			;;
+		-m*)
+			stash_msg=${1#-m}
+			;;
+		--message=*)
+			stash_msg=${1#--message=}
+			;;
 		-u|--include-untracked)
 			shift
 			untracked=${1?"BUG: create_stash () -u requires an argument"}
@@ -193,6 +199,12 @@ store_stash () {
 			shift
 			stash_msg="$1"
 			;;
+		-m*)
+			stash_msg=${1#-m}
+			;;
+		--message=*)
+			stash_msg=${1#--message=}
+			;;
 		-q|--quiet)
 			quiet=t
 			;;
@@ -251,6 +263,12 @@ push_stash () {
 			test -z ${1+x} && usage
 			stash_msg=$1
 			;;
+		-m*)
+			stash_msg=${1#-m}
+			;;
+		--message=*)
+			stash_msg=${1#--message=}
+			;;
 		--help)
 			show_help
 			;;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3b1ac1971..39c7f2ebd 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -804,6 +804,99 @@ test_expect_success 'push -m shows right message' '
 	test_cmp expect actual
 '
 
+test_expect_success 'push -m also works without space' '
+	>foo &&
+	git add foo &&
+	git stash push -m"unspaced test message" &&
+	echo "stash@{0}: On master: unspaced test message" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'store -m foo shows right message' '
+	git stash clear &&
+	git reset --hard &&
+	echo quux >bazzy &&
+	git add bazzy &&
+	STASH_ID=$(git stash create) &&
+	git stash store -m "store m" $STASH_ID &&
+	echo "stash@{0}: store m" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'store -mfoo shows right message' '
+	git stash clear &&
+	git reset --hard &&
+	echo quux >bazzy &&
+	git add bazzy &&
+	STASH_ID=$(git stash create) &&
+	git stash store -m"store mfoo" $STASH_ID &&
+	echo "stash@{0}: store mfoo" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'store --message=foo shows right message' '
+	git stash clear &&
+	git reset --hard &&
+	echo quux >bazzy &&
+	git add bazzy &&
+	STASH_ID=$(git stash create) &&
+	git stash store --message="store message=foo" $STASH_ID &&
+	echo "stash@{0}: store message=foo" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'store --message foo shows right message' '
+	git stash clear &&
+	git reset --hard &&
+	echo quux >bazzy &&
+	git add bazzy &&
+	STASH_ID=$(git stash create) &&
+	git stash store --message "store message foo" $STASH_ID &&
+	echo "stash@{0}: store message foo" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push -mfoo uses right message' '
+	>foo &&
+	git add foo &&
+	git stash push -m"test mfoo" &&
+	echo "stash@{0}: On master: test mfoo" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push --message foo is synonym for -mfoo' '
+	>foo &&
+	git add foo &&
+	git stash push --message "test message foo" &&
+	echo "stash@{0}: On master: test message foo" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push --message=foo is synonym for -mfoo' '
+	>foo &&
+	git add foo &&
+	git stash push --message="test message=foo" &&
+	echo "stash@{0}: On master: test message=foo" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'push -m shows right message' '
+	>foo &&
+	git add foo &&
+	git stash push -m "test m foo" &&
+	echo "stash@{0}: On master: test m foo" >expect &&
+	git stash list -1 >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'create stores correct message' '
 	>foo &&
 	git add foo &&

-- 
2.15.0.471.g17a719cfe.dirty

Jeff King· Nov 22, 2017, 22:01 UTC · re: Phil Hord · lore

Re: [PATCH v2] Teach stash to parse -m/--message like commit does

On Wed, Nov 22, 2017 at 01:20:30PM -0800, Phil Hord wrote:

Show 9 quoted lines

> `git stash push -m foo` uses "foo" as the message for the stash. But
> `git stash push -m"foo"` does not parse successfully.  Similarly
> `git stash push --message="My stash message"` also fails.  The stash
> documentation doesn't suggest this syntax should work, but gitcli
> does and my fingers have learned this pattern long ago for `commit`.
> 
> Teach `git stash` and `git store` to parse -mFoo and --message=Foo
> the same as `git commit` would do.  Even though it's an internal
> function, add similar support to create_stash() for consistency.

I definitely approve of the goal. The implementation looks pretty straightforward given the current parsing scheme.

Many of our other scripts lean on "rev-parse --parseopt" to handle options. E.g.:

OPTIONS="\
git foo [options]
--
m,message=     stash message
"
foo() {
	for i in "$@"; do echo " pre: $i"; done
	eval "$(echo -n "$OPTIONS" | git rev-parse --parseopt -- "$@")"
	for i in "$@"; do echo "post: $i"; done
}
foo -mmsg
foo -m msg
foo --message=msg
foo --message msg

should convert each of those into "-m msg". It also handles unique partial options like "--mess", though IMHO that is not that big a deal.

Would it be possible to convert stash to use --parseopt? I'm fine if the answer is "no", or even "yes, but it's tricky so let's do this in the meantime". But I think that's the endgame we should be shooting for (or, of course, doing the whole thing in C, which I think somebody else is working on).

-Peff

Junio C Hamano· Nov 24, 2017, 05:47 UTC · re: Phil Hord · lore

Re: [PATCH v2] Teach stash to parse -m/--message like commit does

Phil Hord <phil.hord@gmail.com> writes:

Show 9 quoted lines

> `git stash push -m foo` uses "foo" as the message for the stash. But
> `git stash push -m"foo"` does not parse successfully.  Similarly
> `git stash push --message="My stash message"` also fails.  The stash
> documentation doesn't suggest this syntax should work, but gitcli
> does and my fingers have learned this pattern long ago for `commit`.
>
> Teach `git stash` and `git store` to parse -mFoo and --message=Foo
> the same as `git commit` would do.  Even though it's an internal
> function, add similar support to create_stash() for consistency.

I sense some typo around "git store", but I am not exactly sure what the right spelling should be. "git stash -m..", "git stash save -m..", "git stash push -m.." and "git stash store -m..", if you want a full enumeration, but stepping back a bit, mentioning "git stash" ought to be sufficient. A need to spell all of them whose handling of -m you fixed would imply there may be some others whose handling of -m is still broken, which is not a good place for us to end up with.

> Reviewd-by: Junio C Hamano <gitster@pobox.com>

Heh, I didn't review this version and certainly not its test.

I didn't see anything questionable there after a quick read, though.

Thanks.