From: Phillip Wood Date: Mon, 19 Aug 2024 13:57:16 GMT Subject: Re: [PATCH v2] rebase -x: don't print "Executing:" msgs with --quiet Message-ID: <08dc334a-e1d9-4aa1-945e-c543de549163@gmail.com> In-Reply-To: Hi Matheus On 18/08/2024 14:03, Matheus Tavares Bernardino wrote: > On Sat, Aug 17, 2024 at 8:22 AM Junio C Hamano 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. >> 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