Re: [PATCH] status: show commit sha1 in "You are currently cherry-picking" message
On Fri, Oct 11, 2013 at 7:42 PM, Jonathan Nieder wrote: > Ralf Thielow wrote: > >> Especially helpful when cherry-picking multiple commits. > > Neat, thanks. > > [...] >> --- a/t/t7512-status-help.sh >> +++ b/t/t7512-status-help.sh >> @@ -626,9 +626,10 @@ test_expect_success 'prepare for cherry-pick conflicts' >> ' >> test_expect_success 'status when cherry-picking before resolving conflicts' >> ' >> test_when_finished "git cherry-pick --abort" && >> test_must_fail git cherry-pick cherry_branch_second && >> + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) && >> - cat >expected <<\EOF && >> + cat >expected < > Did you mean to drop the '&&'? > No. The important thing was actually the "\" character. Sry > [...] >> @@ -648,11 +649,12 @@ test_expect_success 'status when cherry-picking after >> resolving conflicts' ' >> git reset --hard cherry_branch && >> test_when_finished "git cherry-pick --abort" && >> test_must_fail git cherry-pick cherry_branch_second && >> + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) && >> echo end >main.txt && >> git add main.txt && >> - cat >expected <<\EOF && >> + cat >expected < > Likewise. > > [...] >> --- a/wt-status.c >> +++ b/wt-status.c >> @@ -996,7 +996,8 @@ static void show_cherry_pick_in_progress(struct >> wt_status *s, >> struct wt_status_state *state, >> const char *color) >> { >> - status_printf_ln(s, color, _("You are currently cherry-picking.")); >> + status_printf_ln(s, color, _("You are currently cherry-picking commit >> %s."), >> + find_unique_abbrev(state->cherry_pick_head_sha1, >> DEFAULT_ABBREV)); > > This function is only called when ->cherry_pick_in_progress is true, so > we know cherry_pick_head_sha1 is initialized. Good. > > I would be tempted to check anyway, so that if we ever regress in this, > the cause will be clear and users know to report a bug: > > if (is_null_sha1(state->cherry_pick_head_sha1)) > die("BUG: cherry-pick in progress but no valid > CHERRY_PICK_HEAD?"); > status_printf_ln(s, color, _("You are ... > > I dunno. > > Applied with the && fixes mentioned above on top of the following. > Thanks > -- >8 -- > Subject: status test: add missing && to < > When a test forgets to include && after each command, it is possible > for an early command to succeed but the test to fail, which can hide > bugs. > > Checked using the following patch to the test harness: > > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -425,7 +425,17 @@ test_eval_ () { > eval &3 2>&4 "$*" > } > > +check_command_chaining_ () { > + eval >&3 2>&4 "(exit 189) && $*" > + eval_chain_ret=$? > + if test "$eval_chain_ret" != 189 > + then > + error 'bug in test script: missing "&&" in test > commands' > + fi > +} > + > test_run_ () { > + check_command_chaining_ "$1" > test_cleanup=: > expecting_failure=$2 > setup_malloc_check > > Signed-off-by: Jonathan Nieder > --- > t/t7512-status-help.sh | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh > index 0688d58..9905d43 100755 > --- a/t/t7512-status-help.sh > +++ b/t/t7512-status-help.sh > @@ -669,7 +669,7 @@ EOF > test_expect_success 'status showing detached at and from a tag' ' > test_commit atag tagging && > git checkout atag && > - cat >expected <<\EOF > + cat >expected <<\EOF && > HEAD detached at atag > nothing to commit (use -u to show untracked files) > EOF > @@ -677,7 +677,7 @@ EOF > test_i18ncmp expected actual && > > git reset --hard HEAD^ && > - cat >expected <<\EOF > + cat >expected <<\EOF && > HEAD detached from atag > nothing to commit (use -u to show untracked files) > EOF > @@ -695,7 +695,7 @@ test_expect_success 'status while reverting commit > (conflicts)' ' > test_commit new to-revert.txt && > TO_REVERT=$(git rev-parse --short HEAD^) && > test_must_fail git revert $TO_REVERT && > - cat >expected < + cat >expected < On branch master > You are currently reverting commit $TO_REVERT. >(fix conflicts and run "git revert --continue") > @@ -716,7 +716,7 @@ EOF > test_expect_success 'status while reverting commit (conflicts resolved)' ' > echo reverted >to-revert.txt && > git add to-revert.txt && > - cat >expected < + cat >expected < On branch master > You are currently reverting commit $TO_REVERT. >(all conflicts fixed: run "git revert --continue") > @@ -735,7 +735,7 @@ EOF > > test_expect_success 'status after reverting commit' ' > gi
Re: [PATCH] status: show commit sha1 in "You are currently cherry-picking" message
Ralf Thielow wrote: > Especially helpful when cherry-picking multiple commits. Neat, thanks. [...] > --- a/t/t7512-status-help.sh > +++ b/t/t7512-status-help.sh > @@ -626,9 +626,10 @@ test_expect_success 'prepare for cherry-pick conflicts' ' > test_expect_success 'status when cherry-picking before resolving conflicts' ' > test_when_finished "git cherry-pick --abort" && > test_must_fail git cherry-pick cherry_branch_second && > + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) && > - cat >expected <<\EOF && > + cat >expected < @@ -648,11 +649,12 @@ test_expect_success 'status when cherry-picking after > resolving conflicts' ' > git reset --hard cherry_branch && > test_when_finished "git cherry-pick --abort" && > test_must_fail git cherry-pick cherry_branch_second && > + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) && > echo end >main.txt && > git add main.txt && > - cat >expected <<\EOF && > + cat >expected < --- a/wt-status.c > +++ b/wt-status.c > @@ -996,7 +996,8 @@ static void show_cherry_pick_in_progress(struct wt_status > *s, > struct wt_status_state *state, > const char *color) > { > - status_printf_ln(s, color, _("You are currently cherry-picking.")); > + status_printf_ln(s, color, _("You are currently cherry-picking commit > %s."), > + find_unique_abbrev(state->cherry_pick_head_sha1, > DEFAULT_ABBREV)); This function is only called when ->cherry_pick_in_progress is true, so we know cherry_pick_head_sha1 is initialized. Good. I would be tempted to check anyway, so that if we ever regress in this, the cause will be clear and users know to report a bug: if (is_null_sha1(state->cherry_pick_head_sha1)) die("BUG: cherry-pick in progress but no valid CHERRY_PICK_HEAD?"); status_printf_ln(s, color, _("You are ... I dunno. Applied with the && fixes mentioned above on top of the following. -- >8 -- Subject: status test: add missing && to <&3 2>&4 "$*" } +check_command_chaining_ () { + eval >&3 2>&4 "(exit 189) && $*" + eval_chain_ret=$? + if test "$eval_chain_ret" != 189 + then + error 'bug in test script: missing "&&" in test commands' + fi +} + test_run_ () { + check_command_chaining_ "$1" test_cleanup=: expecting_failure=$2 setup_malloc_check Signed-off-by: Jonathan Nieder --- t/t7512-status-help.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 0688d58..9905d43 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -669,7 +669,7 @@ EOF test_expect_success 'status showing detached at and from a tag' ' test_commit atag tagging && git checkout atag && - cat >expected <<\EOF + cat >expected <<\EOF && HEAD detached at atag nothing to commit (use -u to show untracked files) EOF @@ -677,7 +677,7 @@ EOF test_i18ncmp expected actual && git reset --hard HEAD^ && - cat >expected <<\EOF + cat >expected <<\EOF && HEAD detached from atag nothing to commit (use -u to show untracked files) EOF @@ -695,7 +695,7 @@ test_expect_success 'status while reverting commit (conflicts)' ' test_commit new to-revert.txt && TO_REVERT=$(git rev-parse --short HEAD^) && test_must_fail git revert $TO_REVERT && - cat >expectedexpected expected <<\EOF && On branch master nothing to commit (use -u to show untracked files) EOF -- 1.8.4-50-g437ce60 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: show commit sha1 in "You are currently cherry-picking" message
Ralf Thielow writes: > Especially helpful when cherry-picking multiple commits. I think this would deserve to be in the commit message (but don't consider that blocking). Other than that, looks good to me. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] status: show commit sha1 in "You are currently cherry-picking" message
Signed-off-by: Ralf Thielow --- Especially helpful when cherry-picking multiple commits. t/t7512-status-help.sh | 10 ++ wt-status.c| 7 +-- wt-status.h| 1 + 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh index 0688d58..0a65db1 100755 --- a/t/t7512-status-help.sh +++ b/t/t7512-status-help.sh @@ -626,9 +626,10 @@ test_expect_success 'prepare for cherry-pick conflicts' ' test_expect_success 'status when cherry-picking before resolving conflicts' ' test_when_finished "git cherry-pick --abort" && test_must_fail git cherry-pick cherry_branch_second && - cat >expected <<\EOF && + TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) && + cat >expectedexpected <<\EOF && + cat >expected hints) { if (has_unmerged(s)) status_printf_ln(s, color, @@ -1169,8 +1170,10 @@ void wt_status_get_state(struct wt_status_state *state, state->rebase_in_progress = 1; state->branch = read_and_strip_branch("rebase-merge/head-name"); state->onto = read_and_strip_branch("rebase-merge/onto"); - } else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) { + } else if (!stat(git_path("CHERRY_PICK_HEAD"), &st) && + !get_sha1("CHERRY_PICK_HEAD", sha1)) { state->cherry_pick_in_progress = 1; + hashcpy(state->cherry_pick_head_sha1, sha1); } if (!stat(git_path("BISECT_LOG"), &st)) { state->bisect_in_progress = 1; diff --git a/wt-status.h b/wt-status.h index 9341c56..6c29e6f 100644 --- a/wt-status.h +++ b/wt-status.h @@ -88,6 +88,7 @@ struct wt_status_state { char *detached_from; unsigned char detached_sha1[20]; unsigned char revert_head_sha1[20]; + unsigned char cherry_pick_head_sha1[20]; }; void wt_status_prepare(struct wt_status *s); -- 1.8.4.652.g0d6e0ce -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html