A similar code block is used in three places; this change factors the stash-like check into a separate function so that it can be used in these and other places. Note to reviewers: The existing code in apply_stash appears to assume w_tree, b_tree and i_tree are all trees as opposed to tree-ish (which is what they actually are). This refactoring does not change the existing derivation, but one wonders whether this difference might be subverting the intent of the test which compares c_tree with i_tree. Signed-off-by: Jon Seymour <jon.seymour@xxxxxxxxx> --- git-stash.sh | 46 ++++++++++++++++++++++++++++++---------------- 1 files changed, 30 insertions(+), 16 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 31ea333..dbb7944 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -218,13 +218,30 @@ show_stash () { flags=--stat fi - w_commit=$(git rev-parse --quiet --verify --default $ref_stash "$@") && - b_commit=$(git rev-parse --quiet --verify "$w_commit^") || - die "'$*' is not a stash" + assert_stash_like "$@" git diff $flags $b_commit $w_commit } +# +# if this function returns, then: +# s is set to the stash commit +# w_commit is set to the commit containing the working tree +# b_commit is set to the base commit +# i_commit is set to the commit containing the index tree +# otherwise: +# the function exits via die +# +assert_stash_like() { + # stash records the work tree, and is a merge between the + # base commit (first parent) and the index tree (second parent). + s=$(git rev-parse --revs-only --quiet --verify --default $ref_stash "$@") && + w_commit=$(git rev-parse --quiet --verify "$s:") && + b_commit=$(git rev-parse --quiet --verify "$s^1:") && + i_commit=$(git rev-parse --quiet --verify "$s^2:") || + die "$*: no valid stashed state found" +} + apply_stash () { applied_stash= unstash_index= @@ -253,13 +270,11 @@ apply_stash () { applied_stash="$*" fi - # stash records the work tree, and is a merge between the - # base commit (first parent) and the index tree (second parent). - s=$(git rev-parse --quiet --verify --default $ref_stash "$@") && - w_tree=$(git rev-parse --quiet --verify "$s:") && - b_tree=$(git rev-parse --quiet --verify "$s^1:") && - i_tree=$(git rev-parse --quiet --verify "$s^2:") || - die "$*: no valid stashed state found" + assert_stash_like "$@" + + b_tree=$b_commit + w_tree=$w_commit + i_tree=$i_commit git update-index -q --refresh && git diff-files --quiet --ignore-submodules || @@ -270,6 +285,9 @@ apply_stash () { die 'Cannot apply a stash in the middle of a merge' unstashed_index_tree= + # + # note to reviewers: comparing $c_tree to $i_tree seems unsound, since i_tree is only tree-ish + # if test -n "$unstash_index" && test "$b_tree" != "$i_tree" && test "$c_tree" != "$i_tree" then @@ -351,12 +369,8 @@ drop_stash () { set x "$ref_stash@{0}" shift fi - # Verify supplied argument looks like a stash entry - s=$(git rev-parse --verify "$@") && - git rev-parse --verify "$s:" > /dev/null 2>&1 && - git rev-parse --verify "$s^1:" > /dev/null 2>&1 && - git rev-parse --verify "$s^2:" > /dev/null 2>&1 || - die "$*: not a valid stashed state" + + assert_stash_like "$@" git reflog delete --updateref --rewrite "$@" && say "Dropped $* ($s)" || die "$*: Could not drop stash entry" -- 1.7.2.1.51.g82c0c0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html