On Sat, Aug 17, 2024 at 8:22 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Matheus Tavares <matheus.tavb@xxxxxxxxx> 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.
