Re: [PATCH 4/5] stash: introduce 'git stash store'

[Phil Hord <phil.hord@gmail.com>]

On Fri, Jun 14, 2013 at 6:32 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> 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.

>
>  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_commit

nit: this adds a period to the end of the message which it did not
have previously.

>         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