Re: [PATCH 4/5] stash: introduce 'git stash store'
On Fri, Jun 14, 2013 at 6:32 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
Show 41 quoted lines
> save_stash() contains the logic for doing two potentially independent
> operations; the first is preparing the stash merge commit, and the
> second is updating the stash ref/ reflog accordingly. While the first
> operation is abstracted out into a create_stash() for callers to access
> via 'git stash create', the second one is not. Fix this by factoring
> out the logic for storing the stash into a store_stash() that callers
> can access via 'git stash store'.
>
> Like create, store is not intended for end user interactive use, but for
> callers in other scripts. We can simplify the logic in the
> rebase.autostash feature using this new subcommand.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> Documentation/git-stash.txt | 7 +++++++
> git-stash.sh | 48 +++++++++++++++++++++++++++++++++++++++------
> t/t3903-stash.sh | 19 ++++++++++++++++++
> 3 files changed, 68 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 08cdf2f..ec3a9a3 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -17,6 +17,7 @@ SYNOPSIS
> [-u|--include-untracked] [-a|--all] [<message>]]
> 'git stash' clear
> 'git stash' create [<message>]
> +'git stash' store [-m|--message <message>] [-e|--error <message>] <commit>
>
> DESCRIPTION
> -----------
> @@ -153,6 +154,12 @@ create::
> object name, without storing it anywhere in the ref namespace.
> This is probably not what you want to use; see "save" above.
>
> +store::
> +
> + Store a given stash created via 'git stash create' (which is a
> + dangling merge commit) in the stash ref, updating the stash
> + reflog. This is probably not what you want to use; see
> + "save" above.
Here, again, I think you should explain more about this command. Your commit message explained it pretty well.
+ Like create, store is intended to be useful for scripts.
Show 62 quoted lines
>
> DISCUSSION
> ----------
> diff --git a/git-stash.sh b/git-stash.sh
> index 64800b8..7985473 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -156,6 +156,43 @@ create_stash () {
> die "$(gettext "Cannot record working tree state")"
> }
>
> +store_stash () {
> + while test $# != 0
> + do
> + case "$1" in
> + -m|--message)
> + shift
> + stash_msg="$1"
> + ;;
> + -e|--error)
> + shift
> + error_msg="$1"
> + ;;
> + *)
> + break
> + ;;
> + esac
> + shift
> + done
> + test $# == 1 ||
> + die "$(eval_gettext "\"$dashless store\" requires one <commit> argument")"
> +
> + w_commit="$1"
> + if test -z "$stash_msg"
> + then
> + stash_msg="Created via \"git stash store\"."
> + fi
> + if test -z "$error_msg"
> + then
> + error_msg="Cannot update $ref_stash with $w_commit."
> + fi
> +
> + # Make sure the reflog for stash is kept.
> + : >>"$GIT_DIR/logs/$ref_stash"
> + git update-ref -m "$stash_msg" $ref_stash $w_commit ||
> + die "$(eval_gettext "$error_msg")"
> +}
> +
> save_stash () {
> keep_index=
> patch_mode=
> @@ -227,12 +264,7 @@ save_stash () {
> clear_stash || die "$(gettext "Cannot initialize stash")"
>
> create_stash "$stash_msg" $untracked
> -
> - # Make sure the reflog for stash is kept.
> - : >>"$GIT_DIR/logs/$ref_stash"
> -
> - git update-ref -m "$stash_msg" $ref_stash $w_commit ||
> - die "$(gettext "Cannot save the current status")"
> + store_stash -m "$stash_msg" -e "Cannot save the current status." $w_commitnit: this adds a period to the end of the message which it did not have previously.
Show 44 quoted lines
> say Saved working directory and index state "$stash_msg"
>
> if test -z "$patch_mode"
> @@ -549,6 +581,10 @@ create)
> shift
> create_stash "$*" && echo "$w_commit"
> ;;
> +store)
> + shift
> + store_stash "$@"
> + ;;
> drop)
> shift
> drop_stash "$@"
> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index 5dfbda7..75189ec 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -637,4 +637,23 @@ test_expect_success 'stash where working directory contains "HEAD" file' '
> test_cmp output expect
> '
>
> +test_expect_success 'store called with invalid commit' '
> + test_must_fail git stash store foo
> +'
> +
> +test_expect_success 'store updates stash ref and reflog' '
> + git stash clear &&
> + git reset --hard &&
> + echo quux >bazzy &&
> + git add bazzy &&
> + STASH_ID=$(git stash create) &&
> + git reset --hard &&
> + ! grep quux bazzy &&
> + git stash store -m quuxery $STASH_ID &&
> + test $(cat .git/refs/stash) = $STASH_ID &&
> + grep $STASH_ID .git/logs/refs/stash &&
> + git stash pop &&
> + grep quux bazzy
> +'
> +
> test_done
> --
> 1.8.3.1.383.g0d5ad6b
Looks clean to me, but I didn't test it.
Phil
Ramkumar Ramachandra
