All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Rubén Justo" <rjusto@gmail.com>
To: Junio C Hamano <gitster@pobox.com>, phillip.wood123@gmail.com
Cc: phillip.wood@dunelm.org.uk, Git List <git@vger.kernel.org>,
	Patrick Steinhardt <ps@pks.im>, Jeff King <peff@peff.net>
Subject: Re: [PATCH v5 1/2] add-patch: do not show UI messages on stderr
Date: Wed, 1 May 2024 23:13:03 +0200	[thread overview]
Message-ID: <0dc1f9f9-02ec-4394-9c25-7a7fee6147ee@gmail.com> (raw)
In-Reply-To: <xmqq5xvx5x1u.fsf@gitster.g>

On Wed, May 01, 2024 at 09:14:37AM -0700, Junio C Hamano wrote:
> phillip.wood123@gmail.com 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 ;-).

  reply	other threads:[~2024-05-01 21:13 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-15 19:00 [PATCH] add-patch: response to invalid option Rubén Justo
2024-04-16  5:51 ` Patrick Steinhardt
2024-04-16 19:11   ` Rubén Justo
2024-04-16  9:41 ` phillip.wood123
2024-04-16 19:24   ` Rubén Justo
2024-04-17  9:37     ` phillip.wood123
2024-04-17 15:05       ` Junio C Hamano
2024-04-18 15:11         ` phillip.wood123
2024-04-16 10:26 ` Junio C Hamano
2024-04-16 13:56   ` Phillip Wood
2024-04-16 15:22     ` Junio C Hamano
2024-04-16 15:46       ` Phillip Wood
2024-04-16 16:10         ` Junio C Hamano
2024-04-16 19:31   ` Rubén Justo
2024-04-20 11:08 ` [PATCH v2] add-patch: response to invalid command Rubén Justo
2024-04-20 17:53   ` Junio C Hamano
2024-04-21  9:51   ` [PATCH v3] add-patch: response to unknown command Rubén Justo
2024-04-21 13:18     ` phillip.wood123
2024-04-21 19:37       ` Rubén Justo
2024-04-21 21:52     ` [PATCH v4] " Rubén Justo
2024-04-22 15:50       ` Junio C Hamano
2024-04-24 10:15         ` phillip.wood123
2024-04-24 15:35           ` Junio C Hamano
2024-04-29  9:48             ` Phillip Wood
2024-04-29 16:09               ` Junio C Hamano
2024-04-25  1:44       ` Jeff King
2024-04-25  2:15         ` Eric Sunshine
2024-04-25 20:23           ` Junio C Hamano
2024-04-25 21:00             ` Eric Sunshine
2024-04-25 21:13               ` Junio C Hamano
2024-04-25 21:05             ` Junio C Hamano
2024-04-25 22:09               ` Rubén Justo
2024-04-25 22:16                 ` Junio C Hamano
2024-04-25 23:46                   ` Rubén Justo
2024-04-26  5:39                     ` Junio C Hamano
2024-04-26 16:26                     ` Junio C Hamano
2024-04-26 20:21           ` Jeff King
2024-04-25  3:04         ` Rubén Justo
2024-04-26 20:23           ` Jeff King
2024-04-26 20:41             ` Rubén Justo
2024-04-25  8:53         ` phillip.wood123
2024-04-29 18:35       ` [PATCH v5 0/2] " Rubén Justo
2024-04-29 18:37         ` [PATCH v5 1/2] add-patch: do not show UI messages on stderr Rubén Justo
2024-04-29 19:24           ` Junio C Hamano
2024-04-30 10:52             ` Jeff King
2024-04-30 16:35               ` Rubén Justo
2024-04-30 17:17                 ` Junio C Hamano
2024-04-30 17:11               ` Junio C Hamano
2024-04-30 14:47           ` phillip.wood123
2024-04-30 16:38             ` Rubén Justo
2024-05-01 15:39               ` phillip.wood123
2024-05-01 16:14                 ` Junio C Hamano
2024-05-01 21:13                   ` Rubén Justo [this message]
2024-05-02 16:38                     ` Junio C Hamano
2024-04-29 18:37         ` [PATCH v5 2/2] add-patch: response to unknown command Rubén Justo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0dc1f9f9-02ec-4394-9c25-7a7fee6147ee@gmail.com \
    --to=rjusto@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=phillip.wood123@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.