From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: "Jeff King" <peff@peff.net>, "Rubén Justo" <rjusto@gmail.com>,
"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: Thu, 25 Apr 2024 14:05:33 -0700 [thread overview]
Message-ID: <xmqqv845dug2.fsf@gitster.g> (raw)
In-Reply-To: <xmqqedatfay3.fsf@gitster.g> (Junio C. Hamano's message of "Thu, 25 Apr 2024 13:23:48 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> That's very reminiscent of [1]. Although, unlike [1], the output
>> presented to the user in this case is (I suppose) less likely to be
>> messed up; only the combined captured output is probably affected. So,
>> capturing stdout and stderr separately would indeed be a good idea.
>
> Hmph, something along this line?
>
> It loses to capture how the output should be intermixed, which is
> essential to validate what the end-user should see. As we can see
> in the attached patch, we cannot express that "Unknown ..." should
> come in between two "Stage addition?" questions, which is a downside.
>
> Between adding fflush() before err() writes, and updating err() to
> write to the standard output stream, I am in favor of the latter for
> its simplicity (of the mental model of the resulting code, not of
> the patch that is required to do so).
The latter, which I claimed to prefer, would look like this.
The idea is to perform the interactive session over the standard
output (and the standard input). For that, we teach err() to use
the standard output and have a few fprintf() to also call err().
A few tests expect certain messages to appear on the standard error
stream, which needed adjusting.
I know the previous one "fixes" the CI job at Windows, but I haven't
tried this yet.
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);
va_end(args);
}
@@ -1326,7 +1326,7 @@ static int apply_for_checkout(struct add_p_state *s, struct strbuf *diff,
err(s, _("Nothing was applied.\n"));
} else
/* As a last resort, show the diff to the user */
- fwrite(diff->buf, diff->len, 1, stderr);
+ fwrite(diff->buf, diff->len, 1, stdout);
return 0;
}
@@ -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."));
else if (binary_count == s.file_diff_nr)
- fprintf(stderr, _("Only binary files changed.\n"));
+ err(&s, _("Only binary files changed."));
add_p_state_clear(&s);
return 0;
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 &&
warning: the add.interactive.useBuiltin setting has been removed!
See its entry in '\''git help config'\'' for details.
- No changes.
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 &&
+ 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 21:05 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 [this message]
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
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=xmqqv845dug2.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=phillip.wood@dunelm.org.uk \
--cc=ps@pks.im \
--cc=rjusto@gmail.com \
--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.