Re: [PATCH v2] rebase -x: don't print "Executing:" msgs with --quiet
- From
Matheus Tavares Bernardino <matheus.tavb@gmail.com>
- Date
- Aug 18, 2024, 13:03 UTC
- Message-ID
- <CAGdrTFhZ6KeDPDUoCsV3h5myPuoYf7RR8eFdbFFXGrUGCdEkEw@mail.gmail.com>
- In-Reply-To
- <xmqq34n3jswh.fsf@gitster.g>
On Sat, Aug 17, 2024 at 8:22 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
>
> Matheus Tavares <matheus.tavb@gmail.com> 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?
Show 6 quoted lines
> 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.