Re: [PATCH v5 1/2] add-patch: do not show UI messages on stderr
Hi Rubén
On 30/04/2024 17:38, Rubén Justo wrote:
Show 19 quoted lines
> On Tue, Apr 30, 2024 at 03:47:04PM +0100, phillip.wood123@gmail.com wrote:
>
>>> @@ -1778,9 +1777,9 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
>>> break;
>>> if (s.file_diff_nr == 0)
>>> - fprintf(stderr, _("No changes.\n"));
>>> + err(&s, _("No changes."));
>>> else if (binary_count == s.file_diff_nr)
>>> - fprintf(stderr, _("Only binary files changed.\n"));
>>> + err(&s, _("Only binary files changed."));
>>
>> These two mean we'll now color these messages which we didn't do before. I
>> think if we hit this code we don't print anything else (apart from the
>> warning about add.interactive.useBuiltin being removed) so it probably does
>> not matter whether we use stdout or stderr here and I don't have a strong
>> opinion either way.
>
> Can we consider those messages not part of the UI? IIUC, if we hit that
> code we haven't entered in the interactive UI. Show 20 quoted lines
> Maybe we should:
>
> diff --git a/add-patch.c b/add-patch.c
> index c28ad380ed..b11a435738 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -1780,9 +1780,9 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
> break;
>
> if (s.file_diff_nr == 0)
> - err(&s, _("No changes."));
> + error(_("no changes"));
> else if (binary_count == s.file_diff_nr)
> - err(&s, _("Only binary files changed."));
> + error(_("only binary files changed"));
>
> add_p_state_clear(&s);
> return 0;
>
> Or, simply leave them untouched in this series.Either option sounds good to me
Best Wishes
Phillip