[PATCH v3] rebase --exec: respect --quiet
- From
Matheus Tavares <matheus.tavb@gmail.com>
- Date
- Aug 21, 2024, 01:31 UTC
- Message-ID
- <f105b34b8e6b33448f4d0ef07d51b7bbf4e71aaa.1724203912.git.matheus.tavb@gmail.com>
- In-Reply-To
- <be3c968b0d9085843cd9ce67e85aadfaaafa69c8.1723848510.git.matheus.tavb@gmail.com>
rebase --exec doesn't obey --quiet and ends up printing messages about the command being executed:
git rebase HEAD~3 --quiet --exec true Executing: true Executing: true Executing: true
Let's fix that by omitting the "Executing" messages when using --quiet.
Furthermore, the sequencer code includes a few calls to term_clear_line(), which prints a special character sequence to erase the previous line displayed on stderr (even when nothing was printed yet). For an user running the command interactively, the net effect of calling this function with or without --quiet is the same as the characters are invisible in the terminal. However, when redirecting the output to a file or piping to another command, the presence of these invisible characters is noticeable, and it may break user expectation as --quiet is not being respected.
We could skip the term_clear_line() calls when --quiet is used, like we are doing with the "Executing" messages, but it makes much more sense to condition the line cleaning upon stderr being TTY, since these characters are really only useful for TTY outputs.
The added test checks for both these two changes.
Reported-by: Lincoln Yuji <lincolnyuji@hotmail.com> Reported-by: Rodrigo Siqueira <siqueirajordao@riseup.net> Signed-off-by: Matheus Tavares <matheus.tavb@gmail.com> ---
Changes in v3: - Skipped term_clean_line() when stderr is not a TTY. - Removed the !opts->quiet condition when calling term_clean_line(). - Reworded commit message to better explain the proposed changes.
pager.c | 2 ++ sequencer.c | 7 ++++--- t/t3400-rebase.sh | 6 ++++++ 3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/pager.c b/pager.c index 896f40fcd2..9c24ce6263 100644 --- a/pager.c +++ b/pager.c @@ -234,6 +234,8 @@ int term_columns(void) */ void term_clear_line(void) { + if (!isatty(2)) + return; if (is_terminal_dumb()) /* * Fall back to print a terminal width worth of space diff --git a/sequencer.c b/sequencer.c index 0291920f0b..65c485d783 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3793,12 +3793,13 @@ static int error_failed_squash(struct repository *r, return error_with_patch(r, commit, subject, subject_len, opts, 1, 0); } -static int do_exec(struct repository *r, const char *command_line) +static int do_exec(struct repository *r, const char *command_line, int quiet) { struct child_process cmd = CHILD_PROCESS_INIT; int dirty, status; - fprintf(stderr, _("Executing: %s\n"), command_line); + if (!quiet) + fprintf(stderr, _("Executing: %s\n"), command_line); cmd.use_shell = 1; strvec_push(&cmd.args, command_line); strvec_push(&cmd.env, "GIT_CHERRY_PICK_HELP"); @@ -5013,7 +5014,7 @@ static int pick_commits(struct repository *r, if (!opts->verbose) term_clear_line(); *end_of_arg = '\0'; - res = do_exec(r, arg); + res = do_exec(r, arg, opts->quiet); *end_of_arg = saved; if (res) { diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh index ae34bfad60..bd8bcc381a 100755 --- a/t/t3400-rebase.sh +++ b/t/t3400-rebase.sh @@ -235,6 +235,12 @@ test_expect_success 'rebase --merge -q is quiet' ' test_must_be_empty output.out ' +test_expect_success 'rebase --exec -q is quiet' ' + git checkout -B quiet topic && + git rebase --exec true -q main >output.out 2>&1 && + test_must_be_empty output.out +' + test_expect_success 'Rebase a commit that sprinkles CRs in' ' ( echo "One" &&
-- 2.46.0