{"thread":{"id":"55624","subject":"[PATCH] add: die if both --dry-run and --interactive are given","startedAt":"2021-05-05T14:52:20Z","lastAt":"2021-05-06T21:14:53Z","messageCount":4,"participants":["Øystein Walle","ZheNing Hu","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"423690","messageId":"20210505145204.51614-1-oystwa@gmail.com","threadId":"55624","inReplyTo":null,"subject":"[PATCH] add: die if both --dry-run and --interactive are given","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2021-05-05T14:52:04Z","receivedAt":"2021-05-05T14:52:20Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"The interactive machinery does not obey --dry-run. Die appropriate if\nboth flags are passed.\n\nSigned-off-by: Øystein Walle \n---\nI think a better solution would be to allow this and improve the\ninteractive machinery to handle --dry-run. This is what I could muster\nup at the moment.\n\n builtin/add.c | 2 ++\n t/t3700-add.sh | 4 ++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ea762a41e3..6077eb189f 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -457,6 +457,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (patch_interactive)\n \t\tadd_interactive = 1;\n \tif (add_interactive) {\n+\t\tif (show_only)\n+\t\t\tdie(_(\"--dry-run is incompatible with --interactive/--patch\"));\n \t\tif (pathspec_from_file)\n \t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n \t\texit(interactive_add(argv + 1, prefix, patch_interactive));\ndiff --git a/t/t3700-add.sh b/t/t3700-add.sh\nindex b3b122ff97..171b323f50 100755\n--- a/t/t3700-add.sh\n+++ b/t/t3700-add.sh\n@@ -343,6 +343,10 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out\n \ttest_cmp expect.err actual.err\n '\n \n+test_expect_success 'git add --dry-run --interactive should fail' '\n+\ttest_must_fail git add --dry-run --interactive\n+'\n+\n test_expect_success 'git add empty string should fail' '\n \ttest_must_fail git add \"\"\n '\n-- \n2.27.0\n\n"},{"id":"423725","messageId":"CAOLTT8S5yDNZBkFs8+3sB8vTZQzTBsuTPXpD+d4wiM4ZR9X9QA@mail.gmail.com","threadId":"55624","inReplyTo":"20210505145204.51614-1-oystwa@gmail.com","subject":"Re: [PATCH] add: die if both --dry-run and --interactive are given","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-05-06T01:05:04Z","receivedAt":"2021-05-06T01:05:21Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Hi, Øystein Walle,\n\nØystein Walle 于2021年5月5日周三 下午10:53写道：\n>\n> The interactive machinery does not obey --dry-run. Die appropriate if\n> both flags are passed.\n>\n> Signed-off-by: Øystein Walle \n> ---\n> I think a better solution would be to allow this and improve the\n> interactive machinery to handle --dry-run. This is what I could muster\n> up at the moment.\n>\n> builtin/add.c | 2 ++\n> t/t3700-add.sh | 4 ++++\n> 2 files changed, 6 insertions(+)\n>\n> diff --git a/builtin/add.c b/builtin/add.c\n> index ea762a41e3..6077eb189f 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -457,6 +457,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n> if (patch_interactive)\n> add_interactive = 1;\n> if (add_interactive) {\n> + if (show_only)\n> + die(_(\"--dry-run is incompatible with --interactive/--patch\"));\n> if (pathspec_from_file)\n> die(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n> exit(interactive_add(argv + 1, prefix, patch_interactive));\n> diff --git a/t/t3700-add.sh b/t/t3700-add.sh\n> index b3b122ff97..171b323f50 100755\n> --- a/t/t3700-add.sh\n> +++ b/t/t3700-add.sh\n> @@ -343,6 +343,10 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out\n> test_cmp expect.err actual.err\n> '\n>\n> +test_expect_success 'git add --dry-run --interactive should fail' '\n> + test_must_fail git add --dry-run --interactive\n> +'\n> +\n> test_expect_success 'git add empty string should fail' '\n> test_must_fail git add \"\"\n> '\n> --\n> 2.27.0\n>\n\nThank you for solving this confusion of mine. For the time being,\nit is an easier solution to make the two flags mutually exclusive.\nBut if we need to make `git add -i` support `--dry-run` in the future,\nmaybe we should passed the parameter `DRY_RUN` (maybe an\nenvironment variable) to the sub-process `git -add--interactive.perl`\nand finally this feature may be implemented in `git apply`.\n\nReported-by: ZheNing Hu \n\nThanks!\n--\nZheNing Hu\n"},{"id":"423775","messageId":"20210506141011.18245-1-oystwa@gmail.com","threadId":"55624","inReplyTo":"20210505145204.51614-1-oystwa@gmail.com","subject":"Re: [PATCH] add: die if both --dry-run and --interactive are given","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2021-05-06T14:10:11Z","receivedAt":"2021-05-06T14:10:22Z","isPatch":true,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Hi, Junio and thanks for accepting the patch.\n\n> The interactive machinery does not obey --dry-run. Die appropriate if\n> both flags are passed.\n\nI just noticed a minor spelling error here: \"appropriate\" should be\n\"appropriately\". I can send a v2 if that's easier for you.\n\nØsse\n"},{"id":"423821","messageId":"xmqqczu3y37e.fsf@gitster.g","threadId":"55624","inReplyTo":"20210506141011.18245-1-oystwa@gmail.com","subject":"Re: [PATCH] add: die if both --dry-run and --interactive are given","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-05-06T21:14:45Z","receivedAt":"2021-05-06T21:14:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Øystein Walle writes:\n\n> Hi, Junio and thanks for accepting the patch.\n>\n>> The interactive machinery does not obey --dry-run. Die appropriate if\n>> both flags are passed.\n>\n> I just noticed a minor spelling error here: \"appropriate\" should be\n> \"appropriately\". I can send a v2 if that's easier for you.\n\nThanks, will locally amend---no need to resend.\n\nThanks for contributing.\n"}]}