From: Matheus Tavares Bernardino Date: Sun, 18 Aug 2024 13:03:45 GMT Subject: Re: [PATCH v2] rebase -x: don't print "Executing:" msgs with --quiet Message-ID: In-Reply-To: On Sat, Aug 17, 2024 at 8:22 AM Junio C Hamano wrote: > > Matheus Tavares writes: > > > > > - fprintf(stderr, _("Executing: %s\n"), command_line); > > + if (!quiet) > > + fprintf(stderr, _("Executing: %s\n"), command_line); > > This is very much understandable and match what the proposed log > message explained. > > > @@ -4902,7 +4903,7 @@ static int pick_one_commit(struct repository *r, > > if (item->command == TODO_EDIT) { > > struct commit *commit = item->commit; > > if (!res) { > > - if (!opts->verbose) > > + if (!opts->quiet && !opts->verbose) > > term_clear_line(); > > This is not, though. The original says "if not verbose, clear the > line", so presumably calling the term_clear_line() makes it _less_ > verbose. The reasoning needs to be explained. The idea is that, when running in --quiet mode, we don't want to print anything, not even a line-cleaning char sequence. Nonetheless, since these are invisible chars (assuming we haven't printed anything to be "cleaned" before them), printing them doesn't actually make a difference to the user running rebase in the terminal, as they won't see the chars anyways. The actual issue is when piping/redirecting the rebase output, which will include these invisible chars... So perhaps, instead of modifying the sequencer.c to use "if (!opts->quiet && !opts->verbose) term_clean_line()", the correct approach would be to modify "term_clean_line()" to return earlier "if (!isatty(1))". What do you think? > I actually would have expected that this message ... > > > fprintf(stderr, _("Stopped at %s... %.*s\n"), > > short_commit_name(r, commit), item->arg_len, arg); > > ... goes away when opts->quiet is in effect ;-). Sure, I can add that :) I was mostly focused on the "Executing ..." lines, so that's why I haven't seen/touched this one.