{"thread":{"id":"52440","subject":"[PATCH 00/16] Extend --pathspec-from-file to git add, checkout","startedAt":"2019-12-12T14:36:30Z","lastAt":"2019-12-19T18:03:52Z","messageCount":67,"participants":["Alexandr Miloslavskiy via GitGitGadget","Phillip Wood","Alexandr Miloslavskiy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":16},"messages":[{"id":"388015","messageId":"pull.490.git.1576161385.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":null,"subject":"[PATCH 00/16] Extend --pathspec-from-file to git add, checkout","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:09Z","receivedAt":"2019-12-12T14:36:30Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"This topic continues the effort to support `--pathspec-from-file` in various\ncommands [1][2]. It also includes some refactorings that I developed while\nworking on it - previously submitted separately [3][4] which was probably a\nmistake.\n\nAnatomy of the branch:\n checkout, restore: support the --pathspec-from-file option\n Extends `--pathspec-from-file` to `git checkout/restore`.\n\n t2024: cover more cases\n Some new tests for cases that I deemed worthy while working\n on `parse_branchname_arg()` refactoring.\n\n parse_branchname_arg(): refactor the decision making\n parse_branchname_arg(): update code comments\n parse_branchname_arg(): introduce expect_commit_only\n parse_branchname_arg(): easier to understand variables\n These patches prepare for `|| opts->pathspec_from_file` addition in\n `git checkout/restore` patch. Without this refactoring, I found it\n pretty hard to modify the old code.\n\n checkout: die() on ambiguous tracking branches\n parse_branchname_arg(): extract part as new function\n Initially I was trying to remove some inconsistency standing in the\n way of `git checkout/restore` patch. Later I figured that this change\n is worthy on its own: it prevents some pretty surprising behavior of\n git.\n\n doc: restore: synchronize description\n doc: checkout: synchronize description\n doc: checkout: fix broken text reference\n doc: checkout: remove duplicate synopsis\n Some polishing of docs in preparation for `git checkout/restore` patch.\n\n add: support the --pathspec-from-file option\n cmd_add: prepare for next patch\n Extends `--pathspec-from-file` to `git add`.\n\n commit: forbid --pathspec-from-file --all\n t7107, t7526: directly test parse_pathspec_file()\n Some polishing of merged topic [1].\n\nCC'ing people who shown interest in any of the previous topics, thanks for\nyour reviews!\n\n[1] https://public-inbox.org/git/pull.445.git.1572895605.gitgitgadget@gmail.com/\n[2] https://public-inbox.org/git/20191204203911.237056-1-emilyshaffer@google.com/\n[3] https://public-inbox.org/git/pull.477.git.1574848137.gitgitgadget@gmail.com/\n[4] https://public-inbox.org/git/pull.479.git.1574969538.gitgitgadget@gmail.com/\n\nAlexandr Miloslavskiy (16):\n t7107, t7526: directly test parse_pathspec_file()\n commit: forbid --pathspec-from-file --all\n cmd_add: prepare for next patch\n add: support the --pathspec-from-file option\n doc: checkout: remove duplicate synopsis\n doc: checkout: fix broken text reference\n doc: checkout: synchronize description\n doc: restore: synchronize description\n parse_branchname_arg(): extract part as new function\n checkout: die() on ambiguous tracking branches\n parse_branchname_arg(): easier to understand variables\n parse_branchname_arg(): introduce expect_commit_only\n parse_branchname_arg(): update code comments\n parse_branchname_arg(): refactor the decision making\n t2024: cover more cases\n checkout, restore: support the --pathspec-from-file option\n\n Documentation/git-add.txt | 16 +-\n Documentation/git-checkout.txt | 50 +++--\n Documentation/git-restore.txt | 26 ++-\n Makefile | 1 +\n builtin/add.c | 60 ++++--\n builtin/checkout.c | 279 ++++++++++++++--------------\n builtin/commit.c | 3 +\n t/helper/test-parse-pathspec-file.c | 34 ++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 89 +++++++++\n t/t2024-checkout-dwim.sh | 55 +++++-\n t/t2026-checkout-pathspec-file.sh | 61 ++++++\n t/t2072-restore-pathspec-file.sh | 139 ++++++++++++++\n t/t3704-add-pathspec-file.sh | 49 +++++\n t/t7107-reset-pathspec-file.sh | 105 +----------\n t/t7526-commit-pathspec-file.sh | 80 +-------\n t/t9902-completion.sh | 2 +\n 18 files changed, 694 insertions(+), 357 deletions(-)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n create mode 100755 t/t2026-checkout-pathspec-file.sh\n create mode 100755 t/t2072-restore-pathspec-file.sh\n create mode 100755 t/t3704-add-pathspec-file.sh\n\n\nbase-commit: ad05a3d8e5a6a06443836b5e40434262d992889a\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-490%2FSyntevoAlex%2F%230207_pathspec_from_file_2-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-490/SyntevoAlex/#0207_pathspec_from_file_2-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/490\n-- \ngitgitgadget\n"},{"id":"388016","messageId":"c0980519ed319295c273d358b0547340cc511a6b.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 02/16] commit: forbid --pathspec-from-file --all","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:11Z","receivedAt":"2019-12-12T14:36:31Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nI forgot this in my previous patch `--pathspec-from-file` for\n`git commit` [1]. When both `--pathspec-from-file` and `--all` were\nspecified, `--all` took precedence and `--pathspec-from-file` was\nignored. Before `--pathspec-from-file` was implemented, this case was\nprevented by this check in `parse_and_validate_options()` :\n\n die(_(\"paths '%s ...' with -a does not make sense\"), argv[0]);\n\nIt is unfortunate that these two cases are disconnected. This came as\nresult of how the code was laid out before my patches, where `pathspec`\nis parsed outside of `parse_and_validate_options()`. This branch is\nalready full of refactoring patches and I did not dare to go for another\none.\n\nFix by mirroring `die()` for `--pathspec-from-file` as well.\n\n[1] Commit e440fc58 (\"commit: support the --pathspec-from-file option\" 2019-11-19)\n\nCo-authored-by: Phillip Wood \nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/commit.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 2db2ad0de4..893a9f29b2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -347,6 +347,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\tif (interactive)\n \t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n \n+\t\tif (all)\n+\t\t\tdie(_(\"--pathspec-from-file with -a does not make sense\"));\n+\n \t\tif (pathspec.nr)\n \t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n \n-- \ngitgitgadget\n\n"},{"id":"388017","messageId":"bfdd7a4b5678a7d88c048364db5a5654b1852ca5.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 01/16] t7107, t7526: directly test parse_pathspec_file()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:10Z","receivedAt":"2019-12-12T14:36:33Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nIn my previous patches, `parse_pathspec_file()` was tested indirectly by\ninvoking `git reset` and `git commit` with properly crafted inputs. This\nhas some disadvantages:\n1) A number of tests are copy&pasted for every command where\n `--pathspec-from-file` is supported. With just two commands, it\n wasn't too bad, but I'm going to extend support to many more\n commands, which would make a handful of low-value tests.\n2) Tests are located in suboptimal test packages\n3) Tests are indirect\n\nFix this by testing `parse_pathspec_file()` directly via a new test\nhelper.\n\nWhile working on it, I also noticed that quotes testing via\n`\"\\\"file\\\\101.t\\\"\"` was somewhat incorrect: I escaped `\\` one time while\nI had to escape it two times! Tests still worked due to `\"` which\nprevented pathspec from matching files.\n\nFix this by properly escaping one more time.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Makefile | 1 +\n t/helper/test-parse-pathspec-file.c | 34 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t0067-parse_pathspec_file.sh | 89 +++++++++++++++++++++++\n t/t7107-reset-pathspec-file.sh | 105 ++--------------------------\n t/t7526-commit-pathspec-file.sh | 80 +--------------------\n 7 files changed, 134 insertions(+), 177 deletions(-)\n create mode 100644 t/helper/test-parse-pathspec-file.c\n create mode 100755 t/t0067-parse_pathspec_file.sh\n\ndiff --git a/Makefile b/Makefile\nindex b7d7374dac..fd7bcaac9c 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-mktemp.o\n TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n+TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\ndiff --git a/t/helper/test-parse-pathspec-file.c b/t/helper/test-parse-pathspec-file.c\nnew file mode 100644\nindex 0000000000..e7f525feb9\n--- /dev/null\n+++ b/t/helper/test-parse-pathspec-file.c\n@@ -0,0 +1,34 @@\n+#include \"test-tool.h\"\n+#include \"parse-options.h\"\n+#include \"pathspec.h\"\n+#include \"gettext.h\"\n+\n+int cmd__parse_pathspec_file(int argc, const char **argv)\n+{\n+\tstruct pathspec pathspec;\n+\tconst char *pathspec_from_file = 0;\n+\tint pathspec_file_nul = 0, i;\n+\n+\tstatic const char *const usage[] = {\n+\t\t\"test-tool parse-pathspec-file --pathspec-from-file [--pathspec-file-nul]\",\n+\t\tNULL\n+\t};\n+\n+\tstruct option options[] = {\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n+\t\tOPT_END()\n+\t};\n+\n+\tparse_options(argc, argv, 0, options, usage, 0);\n+\n+\tparse_pathspec_file(&pathspec, 0, 0, 0, pathspec_from_file,\n+\t\t\t pathspec_file_nul);\n+\n+\tfor (i = 0; i < pathspec.nr; i++) {\n+\t\tprintf(\"%s\\n\", pathspec.items[i].original);\n+\t}\n+\n+\tclear_pathspec(&pathspec);\n+\treturn 0;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex f20989d449..c9a232d238 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -39,6 +39,7 @@ static struct test_cmd cmds[] = {\n \t{ \"oidmap\", cmd__oidmap },\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"parse-options\", cmd__parse_options },\n+\t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex 8ed2af71d1..c8549fd87f 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -29,6 +29,7 @@ int cmd__mktemp(int argc, const char **argv);\n int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n+int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\ndiff --git a/t/t0067-parse_pathspec_file.sh b/t/t0067-parse_pathspec_file.sh\nnew file mode 100755\nindex 0000000000..df7b319713\n--- /dev/null\n+++ b/t/t0067-parse_pathspec_file.sh\n@@ -0,0 +1,89 @@\n+#!/bin/sh\n+\n+test_description='Test parse_pathspec_file()'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'one item from stdin' '\n+\techo fileA.t | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'one item from file' '\n+\techo fileA.t >list &&\n+\ttest-tool parse-pathspec-file --pathspec-from-file=list >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\tprintf \"fileA.t\\0fileB.t\\0\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\tprintf \"fileA.t\\nfileB.t\\n\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\tprintf \"fileA.t\\nfileB.t\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tfileB.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'quotes' '\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\t# git takes \\101 and spits A\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tfileA.t\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--pathspec-file-nul takes quotes literally' '\n+\t# shell takes \\\\\\\\101 and spits \\\\101\n+\t# printf takes \\\\101 and spits \\101\n+\tprintf \"\\\"file\\\\\\\\101.t\\\"\" | test-tool parse-pathspec-file --pathspec-from-file=- --pathspec-file-nul >actual &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t\"file\\101.t\"\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_done\ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nindex 6b1a731fff..90dba621d6 100755\n--- a/t/t7107-reset-pathspec-file.sh\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -25,7 +25,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \tgit rm fileA.t &&\n@@ -37,97 +37,21 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t &&\n-\techo fileA.t >list &&\n-\tgit reset --pathspec-from-file=list &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\0fileB.t\\0\" | git reset --pathspec-from-file=- --pathspec-file-nul &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\t D fileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n+test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-\tgit rm fileA.t fileB.t &&\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n \n \tcat >expect <<-\\EOF &&\n-\t D fileA.t\n+\tD fileA.t\n \t D fileB.t\n+\t D fileC.t\n+\tD fileD.t\n \tEOF\n \tverify_expect\n '\n \n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t &&\n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n-\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n-\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n-\n-\tcat >expect <<-\\EOF &&\n-\t D fileA.t\n-\tEOF\n-\ttest_must_fail verify_expect\n-'\n-\n test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n \trestore_checkpoint &&\n \n@@ -137,19 +61,4 @@ test_expect_success '--pathspec-from-file is not compatible with --soft or --har\n \ttest_must_fail git reset --hard --pathspec-from-file=list\n '\n \n-test_expect_success 'only touches what was listed' '\n-\trestore_checkpoint &&\n-\n-\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n-\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tD fileA.t\n-\t D fileB.t\n-\t D fileC.t\n-\tD fileD.t\n-\tEOF\n-\tverify_expect\n-'\n-\n test_done\ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nindex a06b683534..d8fa362ace 100755\n--- a/t/t7526-commit-pathspec-file.sh\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -26,7 +26,7 @@ verify_expect () {\n \ttest_cmp expect actual\n }\n \n-test_expect_success '--pathspec-from-file from stdin' '\n+test_expect_success 'simplest' '\n \trestore_checkpoint &&\n \n \techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n@@ -37,84 +37,6 @@ test_expect_success '--pathspec-from-file from stdin' '\n \tverify_expect\n '\n \n-test_expect_success '--pathspec-from-file from file' '\n-\trestore_checkpoint &&\n-\n-\techo fileA.t >list &&\n-\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'NUL delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'LF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'no trailing delimiter' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'CRLF delimiters' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tA\tfileB.t\n-\tEOF\n-\tverify_expect\n-'\n-\n-test_expect_success 'quotes' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n-\n-\tcat >expect <<-\\EOF &&\n-\tA\tfileA.t\n-\tEOF\n-\tverify_expect expect\n-'\n-\n-test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n-\trestore_checkpoint &&\n-\n-\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n-\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n-'\n-\n test_expect_success 'only touches what was listed' '\n \trestore_checkpoint &&\n \n-- \ngitgitgadget\n\n"},{"id":"388018","messageId":"2d87b6433b8965b833e300b36110b17d936403d1.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 06/16] doc: checkout: fix broken text reference","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:15Z","receivedAt":"2019-12-12T14:36:35Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-checkout.txt | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 2011fdbb1d..d47046e050 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -95,12 +95,10 @@ using `--ours` or `--theirs`. With `-m`, changes made to the working tree\n file can be discarded to re-create the original conflicted merge result.\n \n 'git checkout' (-p|--patch) [] [--] [...]::\n-\tThis is similar to the \"check out paths to the working tree\n-\tfrom either the index or from a tree-ish\" mode described\n-\tabove, but lets you use the interactive interface to show\n-\tthe \"diff\" output and choose which hunks to use in the\n-\tresult. See below for the description of `--patch` option.\n-\n+\tThis is similar to the previous mode, but lets you use the\n+\tinteractive interface to show the \"diff\" output and choose which\n+\thunks to use in the result. See below for the description of\n+\t`--patch` option.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n\n"},{"id":"388019","messageId":"369096494223222ef96e38ca27f0b82c6eb59e92.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 03/16] cmd_add: prepare for next patch","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:12Z","receivedAt":"2019-12-12T14:36:36Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSome code blocks were moved down to be able to test for `pathspec.nr`\nin the next patch. Blocks are moved as is without any changes. This\nis done as separate patch to reduce the amount of diffs in next patch.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/add.c | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex d4686d5218..3d1791dd82 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -430,10 +430,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (addremove && take_worktree_changes)\n \t\tdie(_(\"-A and -u are mutually incompatible\"));\n \n-\tif (!take_worktree_changes && addremove_explicit < 0 && argc)\n-\t\t/* Turn \"git add pathspec...\" to \"git add -A pathspec...\" */\n-\t\taddremove = 1;\n-\n \tif (!show_only && ignore_missing)\n \t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n \n@@ -446,19 +442,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \thold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);\n \n-\tflags = ((verbose ? ADD_CACHE_VERBOSE : 0) |\n-\t\t (show_only ? ADD_CACHE_PRETEND : 0) |\n-\t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n-\t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n-\t\t (!(addremove || take_worktree_changes)\n-\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0));\n-\n-\tif (require_pathspec && argc == 0) {\n-\t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n-\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n-\t\treturn 0;\n-\t}\n-\n \t/*\n \t * Check the \"pathspec '%s' did not match any files\" block\n \t * below before enabling new magic.\n@@ -468,6 +451,23 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t PATHSPEC_SYMLINK_LEADING_PATH,\n \t\t prefix, argv);\n \n+\tif (require_pathspec && argc == 0) {\n+\t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n+\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n+\t\treturn 0;\n+\t}\n+\n+\tif (!take_worktree_changes && addremove_explicit < 0 && argc)\n+\t\t/* Turn \"git add pathspec...\" to \"git add -A pathspec...\" */\n+\t\taddremove = 1;\n+\n+\tflags = ((verbose ? ADD_CACHE_VERBOSE : 0) |\n+\t\t (show_only ? ADD_CACHE_PRETEND : 0) |\n+\t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n+\t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n+\t\t (!(addremove || take_worktree_changes)\n+\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0));\n+\n \tif (read_cache_preload(&pathspec) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n-- \ngitgitgadget\n\n"},{"id":"388020","messageId":"a0a884b384fd7ebdb824bef90acd6d6366be8cf5.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 11/16] parse_branchname_arg(): easier to understand variables","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:20Z","receivedAt":"2019-12-12T14:36:38Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`dash_dash_pos` was only calculated under `opts->accept_pathspec`. This\nis unexpected to readers and made it harder to reason about the code.\nFix this by restoring the expected meaning.\n\nSimplify the code by dropping `argcount` and useless `argc` / `argv`\nmanipulations.\n\nThis should not change behavior in any way.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/checkout.c | 34 ++++++++++++++--------------------\n 1 file changed, 14 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex b847695d2b..f35fe2cc26 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1152,7 +1152,6 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t\t\tstruct object_id *rev)\n {\n \tconst char **new_branch = &opts->new_branch;\n-\tint argcount = 0;\n \tconst char *arg;\n \tint dash_dash_pos;\n \tint has_dash_dash = 0;\n@@ -1211,17 +1210,21 @@ static int parse_branchname_arg(int argc, const char **argv,\n \targ = argv[0];\n \tdash_dash_pos = -1;\n \tfor (i = 0; i < argc; i++) {\n-\t\tif (opts->accept_pathspec && !strcmp(argv[i], \"--\")) {\n+\t\tif (!strcmp(argv[i], \"--\")) {\n \t\t\tdash_dash_pos = i;\n \t\t\tbreak;\n \t\t}\n \t}\n-\tif (dash_dash_pos == 0)\n-\t\treturn 1; /* case (2) */\n-\telse if (dash_dash_pos == 1)\n-\t\thas_dash_dash = 1; /* case (3) or (1) */\n-\telse if (dash_dash_pos >= 2)\n-\t\tdie(_(\"only one reference expected, %d given.\"), dash_dash_pos);\n+\n+\tif (opts->accept_pathspec) {\n+\t if (dash_dash_pos == 0)\n+\t\t return 1; /* case (2) */\n+\t else if (dash_dash_pos == 1)\n+\t\t has_dash_dash = 1; /* case (3) or (1) */\n+\t else if (dash_dash_pos >= 2)\n+\t\t die(_(\"only one reference expected, %d given.\"), dash_dash_pos);\n+\t}\n+\n \topts->count_checkout_paths = !opts->quiet && !has_dash_dash;\n \n \tif (!strcmp(arg, \"-\"))\n@@ -1268,15 +1271,10 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\tif (!recover_with_dwim) {\n \t\t\tif (has_dash_dash)\n \t\t\t\tdie(_(\"invalid reference: %s\"), arg);\n-\t\t\treturn argcount;\n+\t\t\treturn 0;\n \t\t}\n \t}\n \n-\t/* we can't end up being in (2) anymore, eat the argument */\n-\targcount++;\n-\targv++;\n-\targc--;\n-\n \tsetup_new_branch_info_and_source_tree(new_branch_info, opts, rev, arg);\n \n \tif (!opts->source_tree) /* case (1): want a tree */\n@@ -1289,15 +1287,11 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t * even if there happen to be a file called 'branch';\n \t\t * it would be extremely annoying.\n \t\t */\n-\t\tif (argc)\n+\t\tif (argc > 1)\n \t\t\tverify_non_filename(opts->prefix, arg);\n-\t} else if (opts->accept_pathspec) {\n-\t\targcount++;\n-\t\targv++;\n-\t\targc--;\n \t}\n \n-\treturn argcount;\n+\treturn (dash_dash_pos == 1) ? 2 : 1;\n }\n \n static int switch_unborn_to_new_branch(const struct checkout_opts *opts)\n-- \ngitgitgadget\n\n"},{"id":"388021","messageId":"d2f277bb58c6462f1c7f856b5ef2b3ac9894b830.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 08/16] doc: restore: synchronize description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:17Z","receivedAt":"2019-12-12T14:36:39Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`git add` shows an example of good writing, follow it.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-restore.txt | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-restore.txt b/Documentation/git-restore.txt\nindex 1ab2e40ea9..d7bf016bba 100644\n--- a/Documentation/git-restore.txt\n+++ b/Documentation/git-restore.txt\n@@ -8,8 +8,8 @@ git-restore - Restore working tree files\n SYNOPSIS\n --------\n [verse]\n-'git restore' [] [--source=] [--staged] [--worktree] ...\n-'git restore' (-p|--patch) [] [--source=] [--staged] [--worktree] [...]\n+'git restore' [] [--source=] [--staged] [--worktree] [--] ...\n+'git restore' (-p|--patch) [] [--source=] [--staged] [--worktree] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -113,6 +113,14 @@ in linkgit:git-checkout[1] for details.\n \tappear in the `--source` tree are removed, to make them match\n \t`` exactly. The default is no-overlay mode.\n \n+\\--::\n+\tDo not interpret any more arguments as options.\n+\n+...::\n+\tLimits the paths affected by the operation.\n++\n+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].\n+\n EXAMPLES\n --------\n \n-- \ngitgitgadget\n\n"},{"id":"388022","messageId":"271e8ed3ab1a67dbd190ebeb6fe8c675b79b47e3.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 10/16] checkout: die() on ambiguous tracking branches","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:19Z","receivedAt":"2019-12-12T14:36:41Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nBefore this patch, when there were multiple DWIM candidates for remote\nbranch, git decided to try the argument as pathspec instead. I believe\nthat such behavior is a surprise: adding another remote suddenly causes\ngit to discard file contents, because it was unsure which branch to\npick. There was an incomplete attempt to prevent that in [3].\n\nI understand that this was never intended:\n\n [1] introduces the unexpected behavior. Before, there was fallback\n from not-a-ref to pathspec. This is reasonable DWIM. After, there is\n another fallback from ambiguous-remote to pathspec. I understand that\n it was kind of copy&paste oversight.\n\n [2] noticed the unexpected behavior but chose to semi-document it\n instead of forbidding, because the goal of the patch series was\n focused on something else.\n\n [3] adds `die()` when there is ambiguity between branch and file. The\n case of multiple tracking branches is seemingly overlooked.\n\nChange to complain about ambiguity instead of doing unexpected things.\n\n[1] Commit 70c9ac2f (\"DWIM \"git checkout frotz\" to \"git checkout -b frotz origin/frotz\"\" 2009-10-18)\n https://public-inbox.org/git/7vaazpxha4.fsf_-_@alter.siamese.dyndns.org/\n[2] Commit ad8d5104 (\"checkout: add advice for ambiguous \"checkout \"\", 2018-06-05)\n https://public-inbox.org/git/20180502105452.17583-1-avarab@gmail.com/\n[3] Commit be4908f1 (\"checkout: disambiguate dwim tracking branches and local files\", 2018-11-13)\n https://public-inbox.org/git/20181110120707.25846-1-pclouds@gmail.com/\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/checkout.c | 56 ++++++++++++++++++----------------------\n t/t2024-checkout-dwim.sh | 28 ++++++++++++++++++--\n 2 files changed, 51 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex e1b9df1543..b847695d2b 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1115,10 +1115,10 @@ static void setup_new_branch_info_and_source_tree(\n \n static const char *parse_remote_branch(const char *arg,\n \t\t\t\t struct object_id *rev,\n-\t\t\t\t int could_be_checkout_paths,\n-\t\t\t\t int *dwim_remotes_matched)\n+\t\t\t\t int could_be_checkout_paths)\n {\n-\tconst char *remote = unique_tracking_name(arg, rev, dwim_remotes_matched);\n+\tint num_matches = 0;\n+\tconst char *remote = unique_tracking_name(arg, rev, &num_matches);\n \t\n \tif (remote && could_be_checkout_paths) {\n \t\tdie(_(\"'%s' could be both a local file and a tracking branch.\\n\"\n@@ -1126,6 +1126,22 @@ static const char *parse_remote_branch(const char *arg,\n \t\t arg);\n \t}\n \n+\tif (!remote && num_matches > 1) {\n+\t if (advice_checkout_ambiguous_remote_branch_name) {\n+\t\t advise(_(\"If you meant to check out a remote tracking branch on, e.g. 'origin',\\n\"\n+\t\t\t \"you can do so by fully qualifying the name with the --track option:\\n\"\n+\t\t\t \"\\n\"\n+\t\t\t \" git checkout --track origin/\\n\"\n+\t\t\t \"\\n\"\n+\t\t\t \"If you'd like to always have checkouts of an ambiguous prefer\\n\"\n+\t\t\t \"one remote, e.g. the 'origin' remote, consider setting\\n\"\n+\t\t\t \"checkout.defaultRemote=origin in your config.\"));\n+\t }\n+\n+\t die(_(\"'%s' matched multiple (%d) remote tracking branches\"),\n+\t\targ, num_matches);\n+\t}\n+\n \treturn remote;\n }\n \n@@ -1133,8 +1149,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t\t\tint dwim_new_local_branch_ok,\n \t\t\t\tstruct branch_info *new_branch_info,\n \t\t\t\tstruct checkout_opts *opts,\n-\t\t\t\tstruct object_id *rev,\n-\t\t\t\tint *dwim_remotes_matched)\n+\t\t\t\tstruct object_id *rev)\n {\n \tconst char **new_branch = &opts->new_branch;\n \tint argcount = 0;\n@@ -1240,8 +1255,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \n \t\tif (recover_with_dwim) {\n \t\t\tconst char *remote = parse_remote_branch(arg, rev,\n-\t\t\t\t\t\t\t\t could_be_checkout_paths,\n-\t\t\t\t\t\t\t\t dwim_remotes_matched);\n+\t\t\t\t\t\t\t\t could_be_checkout_paths);\n \t\t\tif (remote) {\n \t\t\t\t*new_branch = arg;\n \t\t\t\targ = remote;\n@@ -1505,7 +1519,6 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \t\t\t const char * const usagestr[])\n {\n \tstruct branch_info new_branch_info;\n-\tint dwim_remotes_matched = 0;\n \tint parseopt_flags = 0;\n \n \tmemset(&new_branch_info, 0, sizeof(new_branch_info));\n@@ -1613,8 +1626,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \t\t\topts->track == BRANCH_TRACK_UNSPECIFIED &&\n \t\t\t!opts->new_branch;\n \t\tint n = parse_branchname_arg(argc, argv, dwim_ok,\n-\t\t\t\t\t &new_branch_info, opts, &rev,\n-\t\t\t\t\t &dwim_remotes_matched);\n+\t\t\t\t\t &new_branch_info, opts, &rev);\n \t\targv += n;\n \t\targc -= n;\n \t} else if (!opts->accept_ref && opts->from_treeish) {\n@@ -1672,28 +1684,10 @@ static int checkout_main(int argc, const char **argv, const char *prefix,\n \t}\n \n \tUNLEAK(opts);\n-\tif (opts->patch_mode || opts->pathspec.nr) {\n-\t\tint ret = checkout_paths(opts, new_branch_info.name);\n-\t\tif (ret && dwim_remotes_matched > 1 &&\n-\t\t advice_checkout_ambiguous_remote_branch_name)\n-\t\t\tadvise(_(\"'%s' matched more than one remote tracking branch.\\n\"\n-\t\t\t\t \"We found %d remotes with a reference that matched. So we fell back\\n\"\n-\t\t\t\t \"on trying to resolve the argument as a path, but failed there too!\\n\"\n-\t\t\t\t \"\\n\"\n-\t\t\t\t \"If you meant to check out a remote tracking branch on, e.g. 'origin',\\n\"\n-\t\t\t\t \"you can do so by fully qualifying the name with the --track option:\\n\"\n-\t\t\t\t \"\\n\"\n-\t\t\t\t \" git checkout --track origin/\\n\"\n-\t\t\t\t \"\\n\"\n-\t\t\t\t \"If you'd like to always have checkouts of an ambiguous prefer\\n\"\n-\t\t\t\t \"one remote, e.g. the 'origin' remote, consider setting\\n\"\n-\t\t\t\t \"checkout.defaultRemote=origin in your config.\"),\n-\t\t\t argv[0],\n-\t\t\t dwim_remotes_matched);\n-\t\treturn ret;\n-\t} else {\n+\tif (opts->patch_mode || opts->pathspec.nr)\n+\t\treturn checkout_paths(opts, new_branch_info.name);\n+\telse\n \t\treturn checkout_branch(opts, &new_branch_info);\n-\t}\n }\n \n int cmd_checkout(int argc, const char **argv, const char *prefix)\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex fa0718c730..c35d67b697 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -37,7 +37,9 @@ test_expect_success 'setup' '\n \t\tgit checkout -b foo &&\n \t\ttest_commit a_foo &&\n \t\tgit checkout -b bar &&\n-\t\ttest_commit a_bar\n+\t\ttest_commit a_bar &&\n+\t\tgit checkout -b ambiguous_branch_and_file &&\n+\t\ttest_commit a_ambiguous_branch_and_file\n \t) &&\n \tgit init repo_b &&\n \t(\n@@ -46,7 +48,9 @@ test_expect_success 'setup' '\n \t\tgit checkout -b foo &&\n \t\ttest_commit b_foo &&\n \t\tgit checkout -b baz &&\n-\t\ttest_commit b_baz\n+\t\ttest_commit b_baz &&\n+\t\tgit checkout -b ambiguous_branch_and_file &&\n+\t\ttest_commit b_ambiguous_branch_and_file\n \t) &&\n \tgit remote add repo_a repo_a &&\n \tgit remote add repo_b repo_b &&\n@@ -75,6 +79,26 @@ test_expect_success 'checkout of branch from multiple remotes fails #1' '\n \ttest_branch master\n '\n \n+test_expect_success 'when arg matches multiple remotes, do not fallback to interpreting as pathspec' '\n+\t# create a file with name matching remote branch name\n+\tgit checkout -b t_ambiguous_branch_and_file &&\n+\t>ambiguous_branch_and_file &&\n+\tgit add ambiguous_branch_and_file &&\n+\tgit commit -m \"ambiguous_branch_and_file\" &&\n+\n+\t# modify file to verify that it will not be touched by checkout\n+\ttest_when_finished \"git checkout -- ambiguous_branch_and_file\" &&\n+\techo \"file contents\" >ambiguous_branch_and_file &&\n+\tcp ambiguous_branch_and_file expect &&\n+\n+\ttest_must_fail git checkout ambiguous_branch_and_file 2>err &&\n+\n+\ttest_i18ngrep \"matched multiple (2) remote tracking branches\" err &&\n+\t\n+\t# file must not be altered\n+\ttest_cmp expect ambiguous_branch_and_file\n+'\n+\n test_expect_success 'checkout of branch from multiple remotes fails with advice' '\n \tgit checkout -B master &&\n \ttest_might_fail git branch -D foo &&\n-- \ngitgitgadget\n\n"},{"id":"388023","messageId":"010fd76331f9203c22353cebcfefc63c8d97e4f2.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 14/16] parse_branchname_arg(): refactor the decision making","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:23Z","receivedAt":"2019-12-12T14:36:42Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nMake it easier to understand which branches handle which cases.\n\nDrop obfuscated variable `has_dash_dash` which also took\n`opts->accept_pathspec` into account, making it pretty hard to reason\nabout code, especially when used together with `argc` and\n`opts->accept_pathspec` here:\n\n\tif (!(argc == 1 && !has_dash_dash) &&\n\t\t!(argc == 2 && has_dash_dash) &&\n\t\topts->accept_pathspec)\n\t\trecover_with_dwim = 0;\n\nAvoid double-negation in the code mentioned above (\"it is not OK to\nproceed if it's not one of those cases\").\n\nAvoid hard-to-understand condition `opts->accept_pathspec` in the code\nmentioned above.\n\nThere are some minor die() message changes for:\n`git switch `\n`git switch -- `\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/checkout.c | 74 ++++++++++++++++++++++++++++------------------\n 1 file changed, 45 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 6072f7cef7..aa4ff14ec2 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1153,9 +1153,8 @@ static int parse_branchname_arg(int argc, const char **argv,\n {\n \tconst char **new_branch = &opts->new_branch;\n \tconst char *arg;\n-\tint dash_dash_pos;\n-\tint has_dash_dash = 0, expect_commit_only = 0;\n-\tint i;\n+\tint dash_dash_pos, i;\n+\tint recover_with_dwim, expect_commit_only;\n \n \t/*\n \t * Resolve ambiguity where argv[0] may be or .\n@@ -1174,15 +1173,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t * do that and proceed with (2)(3).\n \t * 5) Otherwise, let caller proceed with interpretation.\n \t */\n-\tif (!argc)\n-\t\treturn 0;\n-\n-\tif (!opts->accept_pathspec) {\n-\t\tif (argc > 1)\n-\t\t\tdie(_(\"only one reference expected\"));\n-\t\thas_dash_dash = 1; /* helps disambiguate */\n-\t}\n-\n+\t \n \targ = argv[0];\n \tdash_dash_pos = -1;\n \tfor (i = 0; i < argc; i++) {\n@@ -1192,17 +1183,49 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t}\n \t}\n \n-\tif (opts->accept_pathspec) {\n-\t if (dash_dash_pos == 0)\n-\t\t return 1;\n-\t else if (dash_dash_pos == 1)\n-\t\t has_dash_dash = 1;\n-\t else if (dash_dash_pos >= 2)\n-\t\t die(_(\"only one reference expected, %d given.\"), dash_dash_pos);\n-\t}\n+\tif (dash_dash_pos == -1) {\n+\t\tif (argc == 0) {\n+\t\t\t/* 'git checkout/switch/restore' */\n+\t\t\treturn 0;\n+\t\t} else if (argc == 1) {\n+\t\t\t/* 'git checkout/switch/restore ' */\n+\t\t\trecover_with_dwim = dwim_new_local_branch_ok;\n+\t\t} else if (!opts->accept_pathspec) {\n+\t\t\t/* 'git switch [...]' */\n+\t\t\tdie(_(\"only one reference expected, %d given.\"), argc);\n+\t\t} else {\n+\t\t\t/* 'git checkout/restore [...]' */\n+\t\t\trecover_with_dwim = 0;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Absence of '--' leaves / ambiguity.\n+\t\t * Try to resolve it with additional knowledge about pathspec args.\n+\t\t */\n+\t\texpect_commit_only = !opts->accept_pathspec;\n+\t} else if (dash_dash_pos == 0) {\n+\t\t/* 'git checkout/switch/restore -- [...]' */\n+\t\treturn 1; /* Eat '--' */\n+\t} else if (dash_dash_pos == 1) {\n+\t\tif (!opts->accept_pathspec) {\n+\t\t\t/* 'git switch -- [...]' */\n+\t\t\tdie(_(\"incompatible with pathspec arguments\"));\n+\t\t}\n+\n+\t\tif (argc == 2) {\n+\t\t\t/* 'git checkout/restore --' */\n+\t\t\trecover_with_dwim = dwim_new_local_branch_ok;\n+\t\t} else {\n+\t\t\t/* 'git checkout/restore -- [...]' */\n+\t\t\trecover_with_dwim = 0;\n+\t\t}\n \n-\tif (has_dash_dash)\n-\t expect_commit_only = 1;\n+\t\t/* Presence of '--' makes it certain that arg is */\n+\t\texpect_commit_only = 1;\n+\t} else {\n+\t\t/* 'git checkout/switch/restore [...] -- [...]' */\n+\t\tdie(_(\"only one reference expected, %d given.\"), dash_dash_pos);\n+\t}\n \n \topts->count_checkout_paths = !opts->quiet && !expect_commit_only;\n \n@@ -1210,19 +1233,12 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\targ = \"@{-1}\";\n \n \tif (get_oid_mb(arg, rev)) {\n-\t\tint recover_with_dwim = dwim_new_local_branch_ok;\n-\n \t\tint could_be_checkout_paths = !expect_commit_only &&\n \t\t\tcheck_filename(opts->prefix, arg);\n \n \t\tif (!expect_commit_only && !no_wildcard(arg))\n \t\t\trecover_with_dwim = 0;\n \n-\t\tif (!(argc == 1 && !has_dash_dash) &&\n-\t\t !(argc == 2 && has_dash_dash) &&\n-\t\t opts->accept_pathspec)\n-\t\t\trecover_with_dwim = 0;\n-\n \t\tif (recover_with_dwim) {\n \t\t\tconst char *remote = parse_remote_branch(arg, rev,\n \t\t\t\t\t\t\t\t could_be_checkout_paths);\n-- \ngitgitgadget\n\n"},{"id":"388024","messageId":"ffbc4059200bda33f8a5fece28b5baf93e465770.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 15/16] t2024: cover more cases","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:24Z","receivedAt":"2019-12-12T14:36:43Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nAfter working on `parse_branchname_arg()` I think that these cases are\nworth testing.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n t/t2024-checkout-dwim.sh | 27 +++++++++++++++++++++++++++\n 1 file changed, 27 insertions(+)\n\ndiff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh\nindex c35d67b697..fd993bf45d 100755\n--- a/t/t2024-checkout-dwim.sh\n+++ b/t/t2024-checkout-dwim.sh\n@@ -156,6 +156,33 @@ test_expect_success 'checkout of branch from a single remote succeeds #2' '\n \ttest_branch_upstream baz repo_b baz\n '\n \n+test_expect_success 'checkout of branch from a single remote succeeds with --' '\n+\tgit checkout -B master &&\n+\ttest_might_fail git branch -D baz &&\n+\n+\tgit checkout baz -- &&\n+\tstatus_uno_is_clean &&\n+\ttest_branch baz &&\n+\ttest_cmp_rev remotes/other_b/baz HEAD &&\n+\ttest_branch_upstream baz repo_b baz\n+'\n+\n+test_expect_success 'dont DWIM with pathspec #1' '\n+\tgit checkout -B master &&\n+\ttest_might_fail git branch -D baz &&\n+\n+\ttest_must_fail git checkout baz nonExistingFile 2>err &&\n+\ttest_i18ngrep \"did not match any file(s) known to git\" err\n+'\n+\n+test_expect_success 'dont DWIM with pathspec #2' '\n+\tgit checkout -B master &&\n+\ttest_might_fail git branch -D baz &&\n+\n+\ttest_must_fail git checkout baz -- nonExistingFile 2>err &&\n+\ttest_i18ngrep \"fatal: invalid reference: baz\" err\n+'\n+\n test_expect_success '--no-guess suppresses branch auto-vivification' '\n \tgit checkout -B master &&\n \tstatus_uno_is_clean &&\n-- \ngitgitgadget\n\n"},{"id":"388025","messageId":"4e2bb6663cdce7fe18e07a5a3713d9ee0c7a7c70.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 13/16] parse_branchname_arg(): update code comments","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:22Z","receivedAt":"2019-12-12T14:36:44Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThese parts repeat git documentation:\n ... if is A...B <...>\n ... remote named in checkout.defaultRemote ...\n\nSome parts repeat the code below. With next patch, code will be easier\nto understand, so this is no longer needed.\n\nThis is a separate patch to reduce the amount of diffs in next patch.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/checkout.c | 86 +++++++++++-----------------------------------\n 1 file changed, 21 insertions(+), 65 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex bd0efa9140..6072f7cef7 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1158,45 +1158,21 @@ static int parse_branchname_arg(int argc, const char **argv,\n \tint i;\n \n \t/*\n-\t * case 1: git checkout -- []\n-\t *\n-\t * must be a valid tree, everything after the '--' must be\n-\t * a path.\n-\t *\n-\t * case 2: git checkout -- []\n-\t *\n-\t * everything after the '--' must be paths.\n-\t *\n-\t * case 3: git checkout [--]\n-\t *\n-\t * (a) If is a commit, that is to\n-\t * switch to the branch or detach HEAD at it. As a special case,\n-\t * if is A...B (missing A or B means HEAD but you can\n-\t * omit at most one side), and if there is a unique merge base\n-\t * between A and B, A...B names that merge base.\n-\t *\n-\t * (b) If is _not_ a commit, either \"--\" is present\n-\t * or is not a path, no -t or -b was given, and\n-\t * and there is a tracking branch whose name is \n-\t * in one and only one remote (or if the branch exists on the\n-\t * remote named in checkout.defaultRemote), then this is a\n-\t * short-hand to fork local from that\n-\t * remote-tracking branch.\n-\t *\n-\t * (c) Otherwise, if \"--\" is present, treat it like case (1).\n-\t *\n-\t * (d) Otherwise :\n-\t * - if it's a reference, treat it like case (1)\n-\t * - else if it's a path, treat it like case (2)\n-\t * - else: fail.\n-\t *\n-\t * case 4: git checkout \n-\t *\n-\t * The first argument must not be ambiguous.\n-\t * - If it's *only* a reference, treat it like case (1).\n-\t * - If it's only a path, treat it like case (2).\n-\t * - else: fail.\n-\t *\n+\t * Resolve ambiguity where argv[0] may be or .\n+\t * High-level approach is:\n+\t * 1) Use various things to reduce ambiguity, examples:\n+\t * * '--' is present\n+\t * * command doesn't accept \n+\t * * additional options like '-b' were given\n+\t * 2) If ambiguous and matches both existing and existing\n+\t * file, complain. However, in 1-argument 'git checkout '\n+\t * treat as to avoid annoying users.\n+\t * 3) Otherwise, if it matches some existing , treat as\n+\t * .\n+\t * 4) Otherwise, if it matches a remote branch, and it's considered\n+\t * reasonable to DWIM to create a local branch from remote branch,\n+\t * do that and proceed with (2)(3).\n+\t * 5) Otherwise, let caller proceed with interpretation.\n \t */\n \tif (!argc)\n \t\treturn 0;\n@@ -1218,9 +1194,9 @@ static int parse_branchname_arg(int argc, const char **argv,\n \n \tif (opts->accept_pathspec) {\n \t if (dash_dash_pos == 0)\n-\t\t return 1; /* case (2) */\n+\t\t return 1;\n \t else if (dash_dash_pos == 1)\n-\t\t has_dash_dash = 1; /* case (3) or (1) */\n+\t\t has_dash_dash = 1;\n \t else if (dash_dash_pos >= 2)\n \t\t die(_(\"only one reference expected, %d given.\"), dash_dash_pos);\n \t}\n@@ -1234,14 +1210,6 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\targ = \"@{-1}\";\n \n \tif (get_oid_mb(arg, rev)) {\n-\t\t/*\n-\t\t * Either case (3) or (4), with not being\n-\t\t * a commit, or an attempt to use case (1) with an\n-\t\t * invalid ref.\n-\t\t *\n-\t\t * It's likely an error, but we need to find out if\n-\t\t * we should auto-create the branch, case (3).(b).\n-\t\t */\n \t\tint recover_with_dwim = dwim_new_local_branch_ok;\n \n \t\tint could_be_checkout_paths = !expect_commit_only &&\n@@ -1250,10 +1218,6 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\tif (!expect_commit_only && !no_wildcard(arg))\n \t\t\trecover_with_dwim = 0;\n \n-\t\t/*\n-\t\t * Accept \"git checkout foo\", \"git checkout foo --\"\n-\t\t * and \"git switch foo\" as candidates for dwim.\n-\t\t */\n \t\tif (!(argc == 1 && !has_dash_dash) &&\n \t\t !(argc == 2 && has_dash_dash) &&\n \t\t opts->accept_pathspec)\n@@ -1265,7 +1229,7 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t\tif (remote) {\n \t\t\t\t*new_branch = arg;\n \t\t\t\targ = remote;\n-\t\t\t\t/* DWIMmed to create local branch, case (3).(b) */\n+\t\t\t\t/* DWIMmed to create local branch */\n \t\t\t} else {\n \t\t\t\trecover_with_dwim = 0;\n \t\t\t}\n@@ -1280,19 +1244,11 @@ static int parse_branchname_arg(int argc, const char **argv,\n \n \tsetup_new_branch_info_and_source_tree(new_branch_info, opts, rev, arg);\n \n-\tif (!opts->source_tree) /* case (1): want a tree */\n+\tif (!opts->source_tree)\n \t\tdie(_(\"reference is not a tree: %s\"), arg);\n \n-\tif (!expect_commit_only) {\t/* case (3).(d) -> (1) */\n-\t\t/*\n-\t\t * Do not complain the most common case\n-\t\t *\tgit checkout branch\n-\t\t * even if there happen to be a file called 'branch';\n-\t\t * it would be extremely annoying.\n-\t\t */\n-\t\tif (argc > 1)\n-\t\t\tverify_non_filename(opts->prefix, arg);\n-\t}\n+\tif (!expect_commit_only && argc > 1)\n+\t\tverify_non_filename(opts->prefix, arg);\n \n \treturn (dash_dash_pos == 1) ? 2 : 1;\n }\n-- \ngitgitgadget\n\n"},{"id":"388026","messageId":"891d1b91cf8f2b5fc8af6f26061bb7967ee6f664.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 09/16] parse_branchname_arg(): extract part as new function","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:18Z","receivedAt":"2019-12-12T14:36:45Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis is done for the next commit to avoid crazy 7x tab code padding.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/checkout.c | 25 +++++++++++++++++++------\n 1 file changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 3634a3dac1..e1b9df1543 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1113,6 +1113,22 @@ static void setup_new_branch_info_and_source_tree(\n \t}\n }\n \n+static const char *parse_remote_branch(const char *arg,\n+\t\t\t\t struct object_id *rev,\n+\t\t\t\t int could_be_checkout_paths,\n+\t\t\t\t int *dwim_remotes_matched)\n+{\n+\tconst char *remote = unique_tracking_name(arg, rev, dwim_remotes_matched);\n+\t\n+\tif (remote && could_be_checkout_paths) {\n+\t\tdie(_(\"'%s' could be both a local file and a tracking branch.\\n\"\n+\t\t\t\"Please use -- (and optionally --no-guess) to disambiguate\"),\n+\t\t arg);\n+\t}\n+\n+\treturn remote;\n+}\n+\n static int parse_branchname_arg(int argc, const char **argv,\n \t\t\t\tint dwim_new_local_branch_ok,\n \t\t\t\tstruct branch_info *new_branch_info,\n@@ -1223,13 +1239,10 @@ static int parse_branchname_arg(int argc, const char **argv,\n \t\t\trecover_with_dwim = 0;\n \n \t\tif (recover_with_dwim) {\n-\t\t\tconst char *remote = unique_tracking_name(arg, rev,\n-\t\t\t\t\t\t\t\t dwim_remotes_matched);\n+\t\t\tconst char *remote = parse_remote_branch(arg, rev,\n+\t\t\t\t\t\t\t\t could_be_checkout_paths,\n+\t\t\t\t\t\t\t\t dwim_remotes_matched);\n \t\t\tif (remote) {\n-\t\t\t\tif (could_be_checkout_paths)\n-\t\t\t\t\tdie(_(\"'%s' could be both a local file and a tracking branch.\\n\"\n-\t\t\t\t\t \"Please use -- (and optionally --no-guess) to disambiguate\"),\n-\t\t\t\t\t arg);\n \t\t\t\t*new_branch = arg;\n \t\t\t\targ = remote;\n \t\t\t\t/* DWIMmed to create local branch, case (3).(b) */\n-- \ngitgitgadget\n\n"},{"id":"388027","messageId":"9af2de98dec456f9314121fc964df037be7f5b36.1576161385.git.gitgitgadget@gmail.com","threadId":"52440","inReplyTo":"pull.490.git.1576161385.gitgitgadget@gmail.com","subject":"[PATCH 16/16] checkout, restore: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-12T14:36:25Z","receivedAt":"2019-12-12T14:36:47Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nDecisions taken for simplicity:\n1) For now, `--pathspec-from-file` is declared incompatible with\n `--patch`, even when is not `stdin`. Such use case it not\n really expected.\n2) It is not allowed to pass pathspec in both args and file.\n\n`you must specify path(s) to restore` block was moved down to be able to\ntest for `pathspec.nr` instead, because testing for `argc` is no longer\ncorrect.\n\n`git switch` does not support the new options because it doesn't expect\n`` arguments.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-checkout.txt | 15 ++++\n Documentation/git-restore.txt | 14 +++\n builtin/checkout.c | 33 +++++--\n t/t2026-checkout-pathspec-file.sh | 61 +++++++++++++\n t/t2072-restore-pathspec-file.sh | 139 ++++++++++++++++++++++++++++++\n t/t9902-completion.sh | 2 +\n 6 files changed, 259 insertions(+), 5 deletions(-)\n create mode 100755 t/t2026-checkout-pathspec-file.sh\n create mode 100755 t/t2072-restore-pathspec-file.sh\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 93124f3ad9..ffe3c1bff2 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -13,6 +13,7 @@ SYNOPSIS\n 'git checkout' [-q] [-f] [-m] [--detach] \n 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] []\n 'git checkout' [-f|--ours|--theirs|-m|--conflict=