From: phillip.wood123@gmail.com
Date: Wed, 01 May 2024 15:39:53 GMT
Subject: Re: [PATCH v5 1/2] add-patch: do not show UI messages on stderr
Message-ID: <7a3fcf9c-a70f-47e8-9624-40b407f90242@gmail.com>
In-Reply-To: <ce11355e-25f3-4d76-91ae-bd561143dd49@gmail.com>

Hi Rubén

On 30/04/2024 17:38, Rubén Justo wrote:
> 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. 

I thikn so

> 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