On Wed, May 01, 2024 at 09:14:37AM -0700, Junio C Hamano wrote:
> phillip.wood123@xxxxxxxxx writes:
>
> >>>> - 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
> > ...
> > I think so
> > ...
> >> - 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
>
> We are returning "success" to the caller, so using error() here is a
> bit strong. Judging from how other messages emitted with err() in
> this program is meant to help the end user, they are all to tell the
> user why their input caused no actual change, and showing these two
> messages the same way as these other messages would be the best for
> consistency.
>
> So I'm inclined to say that what was posted is good. If it paints
> these two messages in the same color as others, that is even getter.
Having:
$ printf "\0" >binary
$ git add -N binary
Displaying the message in RED does not seem as a bad change:
$ git add -i
staged unstaged path
1: +0/-0 binary binary
*** Commands ***
1: status 2: update 3: revert 4: add untracked
5: patch 6: diff 7: quit 8: help
What now> p
<RED>Only binary files changed.<RED>
However, here could be disturbing:
$ git add -p
<RED>Only binary files changed.<RED>
And the "No changes." message is going to have even more audience, I
think.
Perhaps this seems sensible:
Range-diff against v5:
2: b0f042f4ff ! 1: 94c3f80041 add-patch: do not show UI messages on stderr
@@ add-patch.c: int run_add_p(struct repository *r, enum add_p_mode mode,
if (s.file_diff_nr == 0)
- fprintf(stderr, _("No changes.\n"));
-+ err(&s, _("No changes."));
++ fprintf(stdout, _("No changes.\n"));
else if (binary_count == s.file_diff_nr)
- fprintf(stderr, _("Only binary files changed.\n"));
-+ err(&s, _("Only binary files changed."));
++ fprintf(stdout, _("Only binary files changed.\n"));
add_p_state_clear(&s);
return 0;
1: 4a0eb1337f = 2: abaf904e8c add-patch: response to unknown command
But I'm not sure and I do not want to send a new iteration unless it is
necessary. I was already happy with previous versions ;-).
