rebase: fix garbled progress display with '-x'

When running a command with the 'exec' instruction during an interactive rebase session, or for a range of commits using 'git rebase -x', the output can be a bit garbled when the name of the command is short enough: $ git rebase -x true HEAD~5 Executing: true Executing: true Executing: true Executing: true Executing: true) Successfully rebased and updated refs/heads/master. Note the ')' at the end of the last line. It gets more garbled as the range of commits increases: $ git rebase -x true HEAD~50 Executing: true) [ repeated 3 more times ] Executing: true0) [ repeated 44 more times ] Executing: true00) Successfully rebased and updated refs/heads/master. Those extra numbers and ')' are remnants of the previously displayed "Rebasing (N/M)" progress lines that are usually completely overwritten by the "Executing: <cmd>" lines, unless 'cmd' is short and the "N/M" part is long. Make sure that the previously displayed "Rebasing (N/M)" line is cleared by using the term_clear_line() helper function added in the previous patch. Do so only when not being '--verbose', because in that case these "Rebasing (N/M)" lines are not printed as progress (i.e. as lines with '\r' at the end), but as "regular" output (with '\n' at the end). A couple of other rebase commands print similar messages, e.g. "Stopped at <abbrev-oid>... <subject>" for the 'edit' or 'break' commands, or the "Successfully rebased and updated <full-ref>." at the very end. These are so long that they practically always overwrite that "Rebasing (N/M)" progress line, but let's be prudent, and clear the last line before printing these, too. In 't3420-rebase-autostash.sh' two helper functions prepare the expected output of four tests that check the full output of 'git rebase' and thus are affected by this change, so adjust their expectations to account for the new line clearing. Note that this patch doesn't completely eliminate the possibility of similar garbled outputs, e.g. some error messages from rebase or the "Auto-merging <file>" message from within the depths of the merge machinery might not be long enough to completely cover the last "Rebasing (N/M)" line. This patch doesn't do anything about them, because dealing with them individually would result in way too much churn, while having a catch-all term_clear_line() call in the common code path of pick_commits() would hide the "Rebasing (N/M)" line way too soon, and it would either flicker or be invisible. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Jun 27, 2019 at 15:42 UTC d7d90885e0aeb50eb4acc221ee43301b2a43aa3e
2 files changed +16 -5
sequencer.c
+14 -3
@@ -3731,8 +3731,11 @@ static int pick_commits(struct repository *r,
3731 unlink(git_path_merge_head(the_repository));
3732 delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
3733
3734 - if (item->command == TODO_BREAK)
3734 + if (item->command == TODO_BREAK) {
3735 + if (!opts->verbose)
3736 + term_clear_line();
3737 return stopped_at_head(r);
3738 + }
3739 }
3740 if (item->command <= TODO_SQUASH) {
3741 if (is_rebase_i(opts))
@@ -3754,11 +3757,14 @@ static int pick_commits(struct repository *r,
3757 }
3758 if (item->command == TODO_EDIT) {
3759 struct commit *commit = item->commit;
3757 - if (!res)
3760 + if (!res) {
3761 + if (!opts->verbose)
3762 + term_clear_line();
3763 fprintf(stderr,
3764 _("Stopped at %s... %.*s\n"),
3765 short_commit_name(commit),
3766 item->arg_len, arg);
3767 + }
3768 return error_with_patch(r, commit,
3769 arg, item->arg_len, opts, res, !res);
3770 }
@@ -3796,6 +3802,8 @@ static int pick_commits(struct repository *r,
3802 int saved = *end_of_arg;
3803 struct stat st;
3804
3805 + if (!opts->verbose)
3806 + term_clear_line();
3807 *end_of_arg = '\0';
3808 res = do_exec(r, arg);
3809 *end_of_arg = saved;
@@ -3954,10 +3962,13 @@ cleanup_head_ref:
3962 }
3963 apply_autostash(opts);
3964
3957 - if (!opts->quiet)
3965 + if (!opts->quiet) {
3966 + if (!opts->verbose)
3967 + term_clear_line();
3968 fprintf(stderr,
3969 "Successfully rebased and updated %s.\n",
3970 head_ref.buf);
3971 + }
3972
3973 strbuf_release(&buf);
3974 strbuf_release(&head_ref);
t/t3420-rebase-autostash.sh
+2 -2
@@ -49,7 +49,7 @@ create_expected_success_interactive () {
49 $(grep "^Created autostash: [0-9a-f][0-9a-f]*\$" actual)
50 HEAD is now at $(git rev-parse --short feature-branch) third commit
51 Rebasing (1/2)QRebasing (2/2)QApplied autostash.
52 - Successfully rebased and updated refs/heads/rebased-feature-branch.
52 + Q QSuccessfully rebased and updated refs/heads/rebased-feature-branch.
53 EOF
54 }
55
@@ -73,7 +73,7 @@ create_expected_failure_interactive () {
73 Rebasing (1/2)QRebasing (2/2)QApplying autostash resulted in conflicts.
74 Your changes are safe in the stash.
75 You can run "git stash pop" or "git stash drop" at any time.
76 - Successfully rebased and updated refs/heads/rebased-feature-branch.
76 + Q QSuccessfully rebased and updated refs/heads/rebased-feature-branch.
77 EOF
78 }
79