Subject: Re: [PATCH v5 1/2] add-patch: do not show UI messages on stderr

From: Rubén Justo <[email-masked]>
Date: Tue, 30 Apr 2024 18:38:32 +0200

On Tue, Apr 30, 2024 at 03:47:04PM +0100, [email-masked] 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.  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.

> Best Wishes
> Phillip

Thanks.