This topic continues the effort to support `--pathspec-from-file` in various
commands [1][2]. It also includes some refactorings that I developed while
working on it - previously submitted separately [3][4] which was probably a
mistake.
Anatomy of the branch:
checkout, restore: support the --pathspec-from-file option
Extends `--pathspec-from-file` to `git checkout/restore`.
t2024: cover more cases
Some new tests for cases that I deemed worthy while working
on `parse_branchname_arg()` refactoring.
parse_branchname_arg(): refactor the decision making
parse_branchname_arg(): update code comments
parse_branchname_arg(): introduce expect_commit_only
parse_branchname_arg(): easier to understand variables
These patches prepare for `|| opts->pathspec_from_file` addition in
`git checkout/restore` patch. Without this refactoring, I found it
pretty hard to modify the old code.
checkout: die() on ambiguous tracking branches
parse_branchname_arg(): extract part as new function
Initially I was trying to remove some inconsistency standing in the
way of `git checkout/restore` patch. Later I figured that this change
is worthy on its own: it prevents some pretty surprising behavior of
git.
doc: restore: synchronize <pathspec> description
doc: checkout: synchronize <pathspec> description
doc: checkout: fix broken text reference
doc: checkout: remove duplicate synopsis
Some polishing of docs in preparation for `git checkout/restore` patch.
add: support the --pathspec-from-file option
cmd_add: prepare for next patch
Extends `--pathspec-from-file` to `git add`.
commit: forbid --pathspec-from-file --all
t7107, t7526: directly test parse_pathspec_file()
Some polishing of merged topic [1].
CC'ing people who shown interest in any of the previous topics, thanks for
your reviews!
[1] https://public-inbox.org/git/pull.445.git.1572895605.gitgitgadget@xxxxxxxxx/
[2] https://public-inbox.org/git/20191204203911.237056-1-emilyshaffer@xxxxxxxxxx/
[3] https://public-inbox.org/git/pull.477.git.1574848137.gitgitgadget@xxxxxxxxx/
[4] https://public-inbox.org/git/pull.479.git.1574969538.gitgitgadget@xxxxxxxxx/
Changes since V1:
----------------
@Junio please note that V1 was already substantially different from
what you merged into `next`.
1) Added tests for error scenarios related to --pathspec-from-file.
2) Restored tests for --pathspec-file-nul: they are valuable for every
command, because they verify that specific commands handle the
commandline option correctly.
3) Dropped old tests for `git restore` that I forgot to delete when I
made commit `t7107, t7526: directly test parse_pathspec_file()`.
Changes since V2:
----------------
Rebased branch on top of modern master.
Improved code according to code review suggestions in [4] and this topic:
1) Shuffled changes between `parse_branchname_arg()` commits
2) New commit `parse_branchname_arg(): simplify argument eating` with a detailed commit message.
3) Further improved commit `parse_branchname_arg(): easier to understand variables`
4) Fixed an oversight where after refactoring, `parse_branchname_arg()` could eat `--` in `git switch` - this is more of a theoretical problem because `--` is not expected there anyway.
5) Changed aggressive `\-\-` escaping in tests to use `test_i18ngrep -e` instead
Alexandr Miloslavskiy (18):
t7107, t7526: directly test parse_pathspec_file()
t7526: add tests for error conditions
t7107: add tests for error conditions
commit: forbid --pathspec-from-file --all
cmd_add: prepare for next patch
add: support the --pathspec-from-file option
doc: checkout: remove duplicate synopsis
doc: checkout: fix broken text reference
doc: checkout: synchronize <pathspec> description
doc: restore: synchronize <pathspec> description
parse_branchname_arg(): extract part as new function
checkout: die() on ambiguous tracking branches
parse_branchname_arg(): easier to understand variables
parse_branchname_arg(): simplify argument eating
parse_branchname_arg(): update code comments
parse_branchname_arg(): refactor the decision making
t2024: cover more cases
checkout, restore: support the --pathspec-from-file option
Documentation/git-add.txt | 16 +-
Documentation/git-checkout.txt | 50 +++--
Documentation/git-restore.txt | 26 ++-
Makefile | 1 +
builtin/add.c | 60 ++++--
builtin/checkout.c | 277 ++++++++++++++--------------
builtin/commit.c | 3 +
t/helper/test-parse-pathspec-file.c | 34 ++++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/t0067-parse_pathspec_file.sh | 89 +++++++++
t/t2024-checkout-dwim.sh | 55 +++++-
t/t2026-checkout-pathspec-file.sh | 90 +++++++++
t/t2072-restore-pathspec-file.sh | 91 +++++++++
t/t3704-add-pathspec-file.sh | 86 +++++++++
t/t7107-reset-pathspec-file.sh | 98 ++--------
t/t7526-commit-pathspec-file.sh | 83 +++------
t/t9902-completion.sh | 2 +
18 files changed, 742 insertions(+), 321 deletions(-)
create mode 100644 t/helper/test-parse-pathspec-file.c
create mode 100755 t/t0067-parse_pathspec_file.sh
create mode 100755 t/t2026-checkout-pathspec-file.sh
create mode 100755 t/t2072-restore-pathspec-file.sh
create mode 100755 t/t3704-add-pathspec-file.sh
base-commit: 12029dc57db23baef008e77db1909367599210ee
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-490%2FSyntevoAlex%2F%230207_pathspec_from_file_2-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-490/SyntevoAlex/#0207_pathspec_from_file_2-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/490
Range-diff vs v2:
1: 8d5fb9f40b = 1: 8526d1d805 t7107, t7526: directly test parse_pathspec_file()
2: c7cd46d3a3 ! 2: 14d30dd0e1 t7526: add tests for error conditions
@@ -18,22 +18,22 @@
+ >empty_list &&
+
+ test_must_fail git commit --pathspec-from-file=- --interactive -m "Commit" <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-interactive/\-\-patch" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git commit --pathspec-from-file=- --patch -m "Commit" <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-interactive/\-\-patch" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git commit --pathspec-from-file=- -m "Commit" -- fileA.t <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with pathspec arguments" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git commit --pathspec-file-nul -m "Commit" 2>err &&
-+ test_i18ngrep "\-\-pathspec-file-nul requires \-\-pathspec-from-file" err &&
++ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
+
+ test_must_fail git commit --pathspec-from-file=- --include -m "Commit" <empty_list 2>err &&
-+ test_i18ngrep "No paths with \-\-include/\-\-only does not make sense." err &&
++ test_i18ngrep -e "No paths with --include/--only does not make sense." err &&
+
+ test_must_fail git commit --pathspec-from-file=- --only -m "Commit" <empty_list 2>err &&
-+ test_i18ngrep "No paths with \-\-include/\-\-only does not make sense." err
++ test_i18ngrep -e "No paths with --include/--only does not make sense." err
+'
+
test_done
3: b09d74c347 ! 3: b1cc4a960d t7107: add tests for error conditions
@@ -17,13 +17,13 @@
+ echo fileA.t >list &&
+
+ test_must_fail git reset --pathspec-from-file=- --patch <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-patch" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --patch" err &&
+
+ test_must_fail git reset --pathspec-from-file=- -- fileA.t <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with pathspec arguments" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git reset --pathspec-file-nul 2>err &&
-+ test_i18ngrep "\-\-pathspec-file-nul requires \-\-pathspec-from-file" err
++ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err
+'
+
test_done
4: deeb860a85 ! 4: f0be90601e commit: forbid --pathspec-from-file --all
@@ -42,11 +42,11 @@
+++ b/t/t7526-commit-pathspec-file.sh
@@
test_must_fail git commit --pathspec-from-file=- --patch -m "Commit" <list 2>err &&
- test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-interactive/\-\-patch" err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+ test_must_fail git commit --pathspec-from-file=- --all -m "Commit" <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file with \-a does not make sense" err &&
++ test_i18ngrep -e "--pathspec-from-file with -a does not make sense" err &&
+
test_must_fail git commit --pathspec-from-file=- -m "Commit" -- fileA.t <list 2>err &&
- test_i18ngrep "\-\-pathspec-from-file is incompatible with pathspec arguments" err &&
+ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
5: 204a0a4446 = 5: 2153350ac4 cmd_add: prepare for next patch
6: 1ea5f17847 ! 6: c8e59903f7 add: support the --pathspec-from-file option
@@ -186,23 +186,23 @@
+ >empty_list &&
+
+ test_must_fail git add --pathspec-from-file=- --interactive <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-interactive/\-\-patch" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git add --pathspec-from-file=- --patch <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-interactive/\-\-patch" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --interactive/--patch" err &&
+
+ test_must_fail git add --pathspec-from-file=- --edit <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-edit" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --edit" err &&
+
+ test_must_fail git add --pathspec-from-file=- -- fileA.t <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with pathspec arguments" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git add --pathspec-file-nul 2>err &&
-+ test_i18ngrep "\-\-pathspec-file-nul requires \-\-pathspec-from-file" err &&
++ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
+
+ # This case succeeds, but still prints to stderr
+ git add --pathspec-from-file=- <empty_list 2>err &&
-+ test_i18ngrep "Nothing specified, nothing added." err
++ test_i18ngrep -e "Nothing specified, nothing added." err
+'
+
+test_done
7: 3d0fcf6ba5 = 7: 5a4a538fa6 doc: checkout: remove duplicate synopsis
8: 85f7ccc4e0 = 8: daf0d6c536 doc: checkout: fix broken text reference
9: db6e40d004 = 9: 1d981b199f doc: checkout: synchronize <pathspec> description
10: c88cbf453a = 10: 137b697327 doc: restore: synchronize <pathspec> description
11: 2c23bd602d = 11: f53bb13e43 parse_branchname_arg(): extract part as new function
12: efd6876874 = 12: 58b65d2011 checkout: die() on ambiguous tracking branches
14: 2350dc282e ! 13: dd35cad0d9 parse_branchname_arg(): introduce expect_commit_only
@@ -1,8 +1,13 @@
Author: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>
- parse_branchname_arg(): introduce expect_commit_only
+ parse_branchname_arg(): easier to understand variables
- `has_dash_dash` unexpectedly takes `opts->accept_pathspec` into account.
+ `dash_dash_pos` was only calculated under `opts->accept_pathspec`. This
+ is unexpected to readers and made it harder to reason about the code.
+
+ Fix this by restoring the expected meaning.
+
+ `has_dash_dash` also takes `opts->accept_pathspec` into account.
While this may sound clever at first sight, it becomes pretty hard to
reason (and not be a victim) about code, especially in combination with
`argc` here:
@@ -12,10 +17,11 @@
opts->accept_pathspec)
recover_with_dwim = 0;
- Introduce a new non-obfuscated variable to reduce the amount of diffs in
- next patch.
+ Fix this by giving variable a better name and rewriting the above
+ mentioned condition (it's easier to verify if you notice that it only
+ holds when `opts->accept_pathspec` is true).
- This should not change behavior in any way.
+ This patch is not expected to change behavior in any way.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>
@@ -23,23 +29,49 @@
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@
- const char **new_branch = &opts->new_branch;
+ int argcount = 0;
const char *arg;
int dash_dash_pos;
- int has_dash_dash = 0;
-+ int has_dash_dash = 0, expect_commit_only = 0;
++ int arg0_cant_be_pathspec = 0;
int i;
/*
@@
- die(_("only one reference expected, %d given."), dash_dash_pos);
+ if (!opts->accept_pathspec) {
+ if (argc > 1)
+ die(_("only one reference expected"));
+- has_dash_dash = 1; /* helps disambiguate */
++ arg0_cant_be_pathspec = 1;
}
+ arg = argv[0];
+ dash_dash_pos = -1;
+ for (i = 0; i < argc; i++) {
+- if (opts->accept_pathspec && !strcmp(argv[i], "--")) {
++ if (!strcmp(argv[i], "--")) {
+ dash_dash_pos = i;
+ break;
+ }
+ }
+- if (dash_dash_pos == 0)
+- return 1; /* case (2) */
+- else if (dash_dash_pos == 1)
+- has_dash_dash = 1; /* case (3) or (1) */
+- else if (dash_dash_pos >= 2)
+- die(_("only one reference expected, %d given."), dash_dash_pos);
- opts->count_checkout_paths = !opts->quiet && !has_dash_dash;
-+ if (has_dash_dash)
-+ expect_commit_only = 1;
+
-+ opts->count_checkout_paths = !opts->quiet && !expect_commit_only;
++ if (opts->accept_pathspec) {
++ if (dash_dash_pos == 0)
++ return 1; /* case (2) */
++ else if (dash_dash_pos == 1)
++ arg0_cant_be_pathspec = 1; /* case (3) or (1) */
++ else if (dash_dash_pos >= 2)
++ die(_("only one reference expected, %d given."), dash_dash_pos);
++ }
++
++ opts->count_checkout_paths = !opts->quiet && !arg0_cant_be_pathspec;
if (!strcmp(arg, "-"))
arg = "@{-1}";
@@ -48,29 +80,39 @@
int recover_with_dwim = dwim_new_local_branch_ok;
- int could_be_checkout_paths = !has_dash_dash &&
-+ int could_be_checkout_paths = !expect_commit_only &&
++ int could_be_checkout_paths = !arg0_cant_be_pathspec &&
check_filename(opts->prefix, arg);
- if (!has_dash_dash && !no_wildcard(arg))
-+ if (!expect_commit_only && !no_wildcard(arg))
++ if (!arg0_cant_be_pathspec && !no_wildcard(arg))
recover_with_dwim = 0;
/*
+ * Accept "git checkout foo", "git checkout foo --"
+ * and "git switch foo" as candidates for dwim.
+ */
+- if (!(argc == 1 && !has_dash_dash) &&
+- !(argc == 2 && has_dash_dash) &&
++ if (!(argc == 1 && dash_dash_pos == -1) &&
++ !(argc == 2 && dash_dash_pos == 1) &&
+ opts->accept_pathspec)
+ recover_with_dwim = 0;
+
@@
}
if (!recover_with_dwim) {
- if (has_dash_dash)
-+ if (expect_commit_only)
++ if (arg0_cant_be_pathspec)
die(_("invalid reference: %s"), arg);
- return 0;
+ return argcount;
}
@@
if (!opts->source_tree) /* case (1): want a tree */
die(_("reference is not a tree: %s"), arg);
- if (!has_dash_dash) { /* case (3).(d) -> (1) */
-+ if (!expect_commit_only) { /* case (3).(d) -> (1) */
++ if (!arg0_cant_be_pathspec) { /* case (3).(d) -> (1) */
/*
* Do not complain the most common case
* git checkout branch
13: 2498825230 ! 14: dc6e351796 parse_branchname_arg(): easier to understand variables
@@ -1,15 +1,27 @@
Author: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>
- parse_branchname_arg(): easier to understand variables
+ parse_branchname_arg(): simplify argument eating
- `dash_dash_pos` was only calculated under `opts->accept_pathspec`. This
- is unexpected to readers and made it harder to reason about the code.
- Fix this by restoring the expected meaning.
+ This patch abolishes two pieces of obfuscated code.
- Simplify the code by dropping `argcount` and useless `argc` / `argv`
- manipulations.
+ First is `if (argc)` condition, which in fact means "more then one arg"
+ due to `argc++` above.
- This should not change behavior in any way.
+ Second is by far harder to grasp:
+ if (!arg0_cant_be_pathspec) {...} else if (opts->accept_pathspec)
+ that is,
+ if (opts->accept_pathspec && arg0_cant_be_pathspec)
+ which (quite unexpectedly) actually means
+ if (opts->accept_pathspec && dash_dash_pos == 1)
+ and aims to "eat" that `--`.
+
+ Make both pieces easier to read by rewriting obfuscated conditions.
+
+ With both solved, I could keep argcount++/argv++/argc-- in the very end
+ of the function, but that was obviously useless code in this case, so I
+ deleted them as well.
+
+ This patch is not expected to change behavior in any way.
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>
@@ -23,39 +35,10 @@
- int argcount = 0;
const char *arg;
int dash_dash_pos;
- int has_dash_dash = 0;
-@@
- arg = argv[0];
- dash_dash_pos = -1;
- for (i = 0; i < argc; i++) {
-- if (opts->accept_pathspec && !strcmp(argv[i], "--")) {
-+ if (!strcmp(argv[i], "--")) {
- dash_dash_pos = i;
- break;
- }
- }
-- if (dash_dash_pos == 0)
-- return 1; /* case (2) */
-- else if (dash_dash_pos == 1)
-- has_dash_dash = 1; /* case (3) or (1) */
-- else if (dash_dash_pos >= 2)
-- die(_("only one reference expected, %d given."), dash_dash_pos);
-+
-+ if (opts->accept_pathspec) {
-+ if (dash_dash_pos == 0)
-+ return 1; /* case (2) */
-+ else if (dash_dash_pos == 1)
-+ has_dash_dash = 1; /* case (3) or (1) */
-+ else if (dash_dash_pos >= 2)
-+ die(_("only one reference expected, %d given."), dash_dash_pos);
-+ }
-+
- opts->count_checkout_paths = !opts->quiet && !has_dash_dash;
-
- if (!strcmp(arg, "-"))
+ int arg0_cant_be_pathspec = 0;
@@
if (!recover_with_dwim) {
- if (has_dash_dash)
+ if (arg0_cant_be_pathspec)
die(_("invalid reference: %s"), arg);
- return argcount;
+ return 0;
@@ -84,7 +67,7 @@
}
- return argcount;
-+ return (dash_dash_pos == 1) ? 2 : 1;
++ return (opts->accept_pathspec && dash_dash_pos == 1) ? 2 : 1;
}
static int switch_unborn_to_new_branch(const struct checkout_opts *opts)
15: 46f676b8e0 ! 15: 7989f0c5cf parse_branchname_arg(): update code comments
@@ -80,14 +80,14 @@
@@
if (opts->accept_pathspec) {
- if (dash_dash_pos == 0)
-- return 1; /* case (2) */
-+ return 1;
- else if (dash_dash_pos == 1)
-- has_dash_dash = 1; /* case (3) or (1) */
-+ has_dash_dash = 1;
- else if (dash_dash_pos >= 2)
- die(_("only one reference expected, %d given."), dash_dash_pos);
+ if (dash_dash_pos == 0)
+- return 1; /* case (2) */
++ return 1;
+ else if (dash_dash_pos == 1)
+- arg0_cant_be_pathspec = 1; /* case (3) or (1) */
++ arg0_cant_be_pathspec = 1;
+ else if (dash_dash_pos >= 2)
+ die(_("only one reference expected, %d given."), dash_dash_pos);
}
@@
arg = "@{-1}";
@@ -103,17 +103,17 @@
- */
int recover_with_dwim = dwim_new_local_branch_ok;
- int could_be_checkout_paths = !expect_commit_only &&
+ int could_be_checkout_paths = !arg0_cant_be_pathspec &&
@@
- if (!expect_commit_only && !no_wildcard(arg))
+ if (!arg0_cant_be_pathspec && !no_wildcard(arg))
recover_with_dwim = 0;
- /*
- * Accept "git checkout foo", "git checkout foo --"
- * and "git switch foo" as candidates for dwim.
- */
- if (!(argc == 1 && !has_dash_dash) &&
- !(argc == 2 && has_dash_dash) &&
+ if (!(argc == 1 && dash_dash_pos == -1) &&
+ !(argc == 2 && dash_dash_pos == 1) &&
opts->accept_pathspec)
@@
if (remote) {
@@ -132,7 +132,7 @@
+ if (!opts->source_tree)
die(_("reference is not a tree: %s"), arg);
-- if (!expect_commit_only) { /* case (3).(d) -> (1) */
+- if (!arg0_cant_be_pathspec) { /* case (3).(d) -> (1) */
- /*
- * Do not complain the most common case
- * git checkout branch
@@ -142,8 +142,8 @@
- if (argc > 1)
- verify_non_filename(opts->prefix, arg);
- }
-+ if (!expect_commit_only && argc > 1)
++ if (!arg0_cant_be_pathspec && argc > 1)
+ verify_non_filename(opts->prefix, arg);
- return (dash_dash_pos == 1) ? 2 : 1;
+ return (opts->accept_pathspec && dash_dash_pos == 1) ? 2 : 1;
}
16: 319151e4e9 ! 16: e0bdd5bd1e parse_branchname_arg(): refactor the decision making
@@ -34,10 +34,10 @@
const char **new_branch = &opts->new_branch;
const char *arg;
- int dash_dash_pos;
-- int has_dash_dash = 0, expect_commit_only = 0;
+- int arg0_cant_be_pathspec = 0;
- int i;
+ int dash_dash_pos, i;
-+ int recover_with_dwim, expect_commit_only;
++ int recover_with_dwim, arg0_cant_be_pathspec;
/*
* Resolve ambiguity where argv[0] may be <pathspec> or <commit>.
@@ -51,25 +51,22 @@
- if (!opts->accept_pathspec) {
- if (argc > 1)
- die(_("only one reference expected"));
-- has_dash_dash = 1; /* helps disambiguate */
+- arg0_cant_be_pathspec = 1;
- }
--
-+
+
arg = argv[0];
dash_dash_pos = -1;
- for (i = 0; i < argc; i++) {
@@
}
}
- if (opts->accept_pathspec) {
-- if (dash_dash_pos == 0)
-- return 1;
-- else if (dash_dash_pos == 1)
-- has_dash_dash = 1;
-- else if (dash_dash_pos >= 2)
-- die(_("only one reference expected, %d given."), dash_dash_pos);
-- }
+- if (dash_dash_pos == 0)
+- return 1;
+- else if (dash_dash_pos == 1)
+- arg0_cant_be_pathspec = 1;
+- else if (dash_dash_pos >= 2)
+- die(_("only one reference expected, %d given."), dash_dash_pos);
+ if (dash_dash_pos == -1) {
+ if (argc == 0) {
+ /* 'git checkout/switch/restore' */
@@ -89,7 +86,7 @@
+ * Absence of '--' leaves <pathspec>/<commit> ambiguity.
+ * Try to resolve it with additional knowledge about pathspec args.
+ */
-+ expect_commit_only = !opts->accept_pathspec;
++ arg0_cant_be_pathspec = !opts->accept_pathspec;
+ } else if (dash_dash_pos == 0) {
+ /* 'git checkout/switch/restore -- [...]' */
+ return 1; /* Eat '--' */
@@ -106,32 +103,29 @@
+ /* 'git checkout/restore <commit> -- <pathspec> [...]' */
+ recover_with_dwim = 0;
+ }
-
-- if (has_dash_dash)
-- expect_commit_only = 1;
++
+ /* Presence of '--' makes it certain that arg is <commit> */
-+ expect_commit_only = 1;
++ arg0_cant_be_pathspec = 1;
+ } else {
+ /* 'git checkout/switch/restore <commit> <unxpected> [...] -- [...]' */
+ die(_("only one reference expected, %d given."), dash_dash_pos);
-+ }
-
- opts->count_checkout_paths = !opts->quiet && !expect_commit_only;
+ }
+ opts->count_checkout_paths = !opts->quiet && !arg0_cant_be_pathspec;
@@
arg = "@{-1}";
if (get_oid_mb(arg, rev)) {
- int recover_with_dwim = dwim_new_local_branch_ok;
-
- int could_be_checkout_paths = !expect_commit_only &&
+ int could_be_checkout_paths = !arg0_cant_be_pathspec &&
check_filename(opts->prefix, arg);
- if (!expect_commit_only && !no_wildcard(arg))
+ if (!arg0_cant_be_pathspec && !no_wildcard(arg))
recover_with_dwim = 0;
-- if (!(argc == 1 && !has_dash_dash) &&
-- !(argc == 2 && has_dash_dash) &&
+- if (!(argc == 1 && dash_dash_pos == -1) &&
+- !(argc == 2 && dash_dash_pos == 1) &&
- opts->accept_pathspec)
- recover_with_dwim = 0;
-
17: 542eb709ca = 17: 121d3f06a6 t2024: cover more cases
18: c293d72832 ! 18: 7324e091ba checkout, restore: support the --pathspec-from-file option
@@ -105,8 +105,8 @@
* Absence of '--' leaves <pathspec>/<commit> ambiguity.
* Try to resolve it with additional knowledge about pathspec args.
*/
-- expect_commit_only = !opts->accept_pathspec;
-+ expect_commit_only = !opts->accept_pathspec || opts->pathspec_from_file;
+- arg0_cant_be_pathspec = !opts->accept_pathspec;
++ arg0_cant_be_pathspec = !opts->accept_pathspec || opts->pathspec_from_file;
} else if (dash_dash_pos == 0) {
/* 'git checkout/switch/restore -- [...]' */
return 1; /* Eat '--' */
@@ -247,16 +247,16 @@
+ echo fileA.t >list &&
+
+ test_must_fail git checkout --pathspec-from-file=- --detach <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-detach" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --detach" err &&
+
+ test_must_fail git checkout --pathspec-from-file=- --patch <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-patch" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --patch" err &&
+
+ test_must_fail git checkout --pathspec-from-file=- -- fileA.t <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with pathspec arguments" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git checkout --pathspec-file-nul 2>err &&
-+ test_i18ngrep "\-\-pathspec-file-nul requires \-\-pathspec-from-file" err
++ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err
+'
+
+test_done
@@ -344,16 +344,16 @@
+ >empty_list &&
+
+ test_must_fail git restore --pathspec-from-file=- --patch --source=HEAD^1 <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with \-\-patch" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with --patch" err &&
+
+ test_must_fail git restore --pathspec-from-file=- --source=HEAD^1 -- fileA.t <list 2>err &&
-+ test_i18ngrep "\-\-pathspec-from-file is incompatible with pathspec arguments" err &&
++ test_i18ngrep -e "--pathspec-from-file is incompatible with pathspec arguments" err &&
+
+ test_must_fail git restore --pathspec-file-nul --source=HEAD^1 2>err &&
-+ test_i18ngrep "\-\-pathspec-file-nul requires \-\-pathspec-from-file" err &&
++ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
+
+ test_must_fail git restore --pathspec-from-file=- --source=HEAD^1 <empty_list 2>err &&
-+ test_i18ngrep "you must specify path(s) to restore" err
++ test_i18ngrep -e "you must specify path(s) to restore" err
+'
+
+test_done
--
gitgitgadget
