From: "Rubén Justo" <rjusto@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
Eric Sunshine <sunshine@sunshineco.com>
Cc: Jeff King <peff@peff.net>, Git List <git@vger.kernel.org>,
Phillip Wood <phillip.wood@dunelm.org.uk>,
Patrick Steinhardt <ps@pks.im>
Subject: Re: [PATCH v4] add-patch: response to unknown command
Date: Fri, 26 Apr 2024 00:09:20 +0200 [thread overview]
Message-ID: <5854e33c-b84e-47a3-94c6-c5fe8c9a4c98@gmail.com> (raw)
In-Reply-To: <xmqqv845dug2.fsf@gitster.g>
On Thu, Apr 25, 2024 at 02:05:33PM -0700, Junio C Hamano wrote:
I am assuming that this change will precede my series.
I leave some nits below.
> diff --git c/add-patch.c w/add-patch.c
> index 7be142d448..c28ad380ed 100644
> --- c/add-patch.c
> +++ w/add-patch.c
> @@ -293,10 +293,10 @@ static void err(struct add_p_state *s, const char *fmt, ...)
> va_list args;
>
> va_start(args, fmt);
> - fputs(s->s.error_color, stderr);
> - vfprintf(stderr, fmt, args);
> - fputs(s->s.reset_color, stderr);
> - fputc('\n', stderr);
> + fputs(s->s.error_color, stdout);
> + vfprintf(stdout, fmt, args);
> + fputs(s->s.reset_color, stdout);
> + fputc('\n', stdout);
If we're going to use stdout, perhaps we can be less explicit?
diff --git a/add-patch.c b/add-patch.c
index 447e8166d2..a204224dae 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -293,10 +293,10 @@ static void err(struct add_p_state *s, const char *fmt, ...)
va_list args;
va_start(args, fmt);
- fputs(s->s.error_color, stderr);
- vfprintf(stderr, fmt, args);
- fputs(s->s.reset_color, stderr);
- fputc('\n', stderr);
+ puts(s->s.error_color);
+ vprintf(fmt, args);
+ puts(s->s.reset_color);
+ putchar('\n');
va_end(args);
}
> @@ -1780,9 +1780,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."));
Nice.
> else if (binary_count == s.file_diff_nr)
> - fprintf(stderr, _("Only binary files changed.\n"));
> + err(&s, _("Only binary files changed."));
Nice.
> diff --git c/t/t3701-add-interactive.sh w/t/t3701-add-interactive.sh
> index 482d5c117e..a315ec99a3 100755
> --- c/t/t3701-add-interactive.sh
> +++ w/t/t3701-add-interactive.sh
> @@ -43,17 +43,17 @@ force_color () {
> }
>
> test_expect_success 'warn about add.interactive.useBuiltin' '
> - cat >expect <<-\EOF &&
> + cat >expect.error <<-\EOF &&
This expect.error is what we are going to test with the output on
stderr ...
> warning: the add.interactive.useBuiltin setting has been removed!
> See its entry in '\''git help config'\'' for details.
> - No changes.
however, this line no longer goes to stderr. OK.
> EOF
>
> for v in = =true =false
> do
> - git -c "add.interactive.useBuiltin$v" add -p >out 2>actual &&
> - test_must_be_empty out &&
> - test_cmp expect actual || return 1
> + git -c "add.interactive.useBuiltin$v" add -p >actual 2>error &&
> + echo "No changes." >expect &&
Why not prepare this file above, out of the loop?
> + test_cmp expect actual &&
> + test_cmp expect.error error || return 1
> done
> '
>
> @@ -348,13 +348,13 @@ test_expect_success 'different prompts for mode change/deleted' '
>
> test_expect_success 'correct message when there is nothing to do' '
> git reset --hard &&
> - git add -p 2>err &&
> - test_grep "No changes" err &&
> + git add -p >out &&
> + test_grep "No changes" out &&
> printf "\\0123" >binary &&
> git add binary &&
> printf "\\0abc" >binary &&
> - git add -p 2>err &&
> - test_grep "Only binary files changed" err
> + git add -p >out &&
> + test_grep "Only binary files changed" out
> '
>
> test_expect_success 'setup again' '
next prev parent reply other threads:[~2024-04-25 22:10 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 [this message]
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
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=5854e33c-b84e-47a3-94c6-c5fe8c9a4c98@gmail.com \
--to=rjusto@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
--cc=ps@pks.im \
--cc=sunshine@sunshineco.com \
/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.