From 9d225b025d96d37e8c914646c54cfa9fb5f52b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Mon, 29 Apr 2024 20:37:03 +0200 Subject: [PATCH 1/2] add-patch: do not show UI messages on stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is no need to show some UI messages on stderr, and yet doing so may produce some undesirable results, such as messages appearing in an unexpected order. Let's use stdout for all UI messages, and adjusts the tests accordingly. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- add-patch.c | 13 ++++++------- t/t3701-add-interactive.sh | 12 ++++++------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/add-patch.c b/add-patch.c index a06dd18985..43abd57ef6 100644 --- a/add-patch.c +++ b/add-patch.c @@ -293,10 +293,9 @@ 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); + vprintf(fmt, args); + puts(s->s.reset_color); va_end(args); } @@ -1326,7 +1325,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; } @@ -1777,9 +1776,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 a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index bc55255b0a..4d4eaca9a1 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -44,13 +44,13 @@ test_expect_success 'warn about add.interactive.useBuiltin' ' cat >expect <<-\EOF && warning: the add.interactive.useBuiltin setting has been removed! See its entry in '\''git help config'\'' for details. - No changes. EOF + echo "No changes." >expect.out && 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.out out && test_cmp expect actual || return 1 done ' @@ -334,13 +334,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' ' From 26998ed2a2ed2e36103da7131e44cd6a0c8d4565 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Mon, 29 Apr 2024 20:37:12 +0200 Subject: [PATCH 2/2] add-patch: response to unknown command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the user gives an unknown command to the "add -p" prompt, the list of accepted commands with their explanation is given. This is the same output they get when they say '?'. However, the unknown command may be due to a user input error rather than the user not knowing the valid command. To reduce the likelihood of user confusion and error repetition, instead of displaying the list of accepted commands, display a short error message with the unknown command received, as feedback to the user. Include a reminder about the current command '?' in the new message, to guide the user if they want help. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano --- add-patch.c | 5 ++++- t/t3701-add-interactive.sh | 16 +++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/add-patch.c b/add-patch.c index 43abd57ef6..85c4770572 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1666,7 +1666,7 @@ soft_increment: } } else if (s->answer.buf[0] == 'p') { rendered_hunk_index = -1; - } else { + } else if (s->answer.buf[0] == '?') { const char *p = _(help_patch_remainder), *eol = p; color_fprintf(stdout, s->s.help_color, "%s", @@ -1690,6 +1690,9 @@ soft_increment: color_fprintf_ln(stdout, s->s.help_color, "%.*s", (int)(eol - p), p); } + } else { + err(s, _("Unknown command '%s' (use '?' for help)"), + s->answer.buf); } } diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 4d4eaca9a1..e9b3b28761 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -7,6 +7,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh +SP=" " + diff_cmp () { for x do @@ -55,6 +57,19 @@ test_expect_success 'warn about add.interactive.useBuiltin' ' done ' +test_expect_success 'unknown command' ' + test_when_finished "git reset --hard; rm -f command" && + echo W >command && + git add -N command && + git diff command >expect && + cat >>expect <<-EOF && + (1/1) Stage addition [y,n,q,a,d,e,p,?]? Unknown command ${SQ}W${SQ} (use ${SQ}?${SQ} for help) + (1/1) Stage addition [y,n,q,a,d,e,p,?]?$SP + EOF + git add -p -- command actual 2>&1 && + test_cmp expect actual +' + test_expect_success 'setup (initial)' ' echo content >file && git add file && @@ -231,7 +246,6 @@ test_expect_success 'setup file' ' ' test_expect_success 'setup patch' ' - SP=" " && NULL="" && cat >patch <<-EOF @@ -1,4 +1,4 @@