* [PATCH v2] Teach stash to parse -m/--message like commit does
@ 2017-11-22 21:20 Phil Hord
2017-11-22 22:01 ` Jeff King
2017-11-24 5:47 ` Junio C Hamano
0 siblings, 2 replies; 3+ messages in thread
From: Phil Hord @ 2017-11-22 21:20 UTC (permalink / raw)
To: Git; +Cc: Thomas Gummerer, Phil Hord
`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(+)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Teach stash to parse -m/--message like commit does
2017-11-22 21:20 [PATCH v2] Teach stash to parse -m/--message like commit does Phil Hord
@ 2017-11-22 22:01 ` Jeff King
2017-11-24 5:47 ` Junio C Hamano
1 sibling, 0 replies; 3+ messages in thread
From: Jeff King @ 2017-11-22 22:01 UTC (permalink / raw)
To: Phil Hord; +Cc: Git, Thomas Gummerer
On Wed, Nov 22, 2017 at 01:20:30PM -0800, Phil Hord wrote:
> `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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] Teach stash to parse -m/--message like commit does
2017-11-22 21:20 [PATCH v2] Teach stash to parse -m/--message like commit does Phil Hord
2017-11-22 22:01 ` Jeff King
@ 2017-11-24 5:47 ` Junio C Hamano
1 sibling, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2017-11-24 5:47 UTC (permalink / raw)
To: Phil Hord; +Cc: Git, Thomas Gummerer
Phil Hord <phil.hord@gmail.com> writes:
> `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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-11-24 5:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-22 21:20 [PATCH v2] Teach stash to parse -m/--message like commit does Phil Hord
2017-11-22 22:01 ` Jeff King
2017-11-24 5:47 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).