Re: [PATCH v2] rebase -x: don't print "Executing:" msgs with --quiet

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Aug 19, 2024, 13:57 UTC
Message-ID: <08dc334a-e1d9-4aa1-945e-c543de549163@gmail.com>
In-Reply-To: <CAGdrTFhZ6KeDPDUoCsV3h5myPuoYf7RR8eFdbFFXGrUGCdEkEw@mail.gmail.com>

Hi Matheus

On 18/08/2024 14:03, Matheus Tavares Bernardino wrote:

Show 15 quoted lines

> On Sat, Aug 17, 2024 at 8:22 AM Junio C Hamano <gitster@pobox.com> wrote:
> 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?

On the face of it that sounds like a good idea but I haven't thought too much about it. These messages are all going to stderr rather than stdout. If we do go that way we'll need to adjust launch_specified_editor() in editor.c to either suppress the hint or terminate it with '\n' if stderr is not a terminal.

Show 9 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.

If we're going to suppress this we should probably suppress the message about amending the commit that gets printed after this by error_with_patch(). There are a number of other places that we ignore "--quiet". stopped_at_head() prints a similar message to the one above when we stop for a "break" command and currently ignores "--quiet". Should the messages from "--autostash" be suppressed by "--quiet"? What about when a commit is dropped because it is has become empty in do_pick_commit()?

Thanks for working on this, it would be nice to have the sequencer respect "--quiet" better.

Phillip