rm: support the --pathspec-from-file option
Decisions taken for simplicity:
1) It is not allowed to pass pathspec in both args and file.
Adjustments were needed for `if (!argc)` block:
This code actually means "pathspec is not present". Previously, pathspec
could only come from commandline arguments, so testing for `argc` was a
valid way of testing for the presence of pathspec. But this is no longer
true with `--pathspec-from-file`.
During the entire `--pathspec-from-file` story, I tried to keep its
behavior very close to giving pathspec on commandline, so that switching
from one to another doesn't involve any surprises.
However, throwing usage at user in the case of empty
`--pathspec-from-file` would puzzle because there's nothing wrong with
"usage" (that is, argc/argv array).
On the other hand, throwing usage in the old case also feels bad to me.
While it's less of a puzzle, I (as user) never liked the experience of
comparing my commandline to "usage", trying to spot a difference. Since
it's already known what the error is, it feels a lot better to give that
specific error to user.
Judging from [1] it doesn't seem that showing usage in this case was
important (the patch was to avoid segfault), and it doesn't fit into how
other commands react to empty pathspec (see for example `git add` with a
custom message).
Therefore, I decided to show new error text in both cases. In order to
continue testing for error early, I moved `parse_pathspec()` higher. Now
it happens before `read_cache()` / `hold_locked_index()` /
`setup_work_tree()`, which shouldn't cause any issues.
[1] Commit 7612a1ef ("git-rm: honor -n flag" 2006-06-09)
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
5 years ago
|
|
|
#!/bin/sh
|
|
|
|
|
|
|
|
test_description='rm --pathspec-from-file'
|
|
|
|
|
|
|
|
TEST_PASSES_SANITIZE_LEAK=true
|
rm: support the --pathspec-from-file option
Decisions taken for simplicity:
1) It is not allowed to pass pathspec in both args and file.
Adjustments were needed for `if (!argc)` block:
This code actually means "pathspec is not present". Previously, pathspec
could only come from commandline arguments, so testing for `argc` was a
valid way of testing for the presence of pathspec. But this is no longer
true with `--pathspec-from-file`.
During the entire `--pathspec-from-file` story, I tried to keep its
behavior very close to giving pathspec on commandline, so that switching
from one to another doesn't involve any surprises.
However, throwing usage at user in the case of empty
`--pathspec-from-file` would puzzle because there's nothing wrong with
"usage" (that is, argc/argv array).
On the other hand, throwing usage in the old case also feels bad to me.
While it's less of a puzzle, I (as user) never liked the experience of
comparing my commandline to "usage", trying to spot a difference. Since
it's already known what the error is, it feels a lot better to give that
specific error to user.
Judging from [1] it doesn't seem that showing usage in this case was
important (the patch was to avoid segfault), and it doesn't fit into how
other commands react to empty pathspec (see for example `git add` with a
custom message).
Therefore, I decided to show new error text in both cases. In order to
continue testing for error early, I moved `parse_pathspec()` higher. Now
it happens before `read_cache()` / `hold_locked_index()` /
`setup_work_tree()`, which shouldn't cause any issues.
[1] Commit 7612a1ef ("git-rm: honor -n flag" 2006-06-09)
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
5 years ago
|
|
|
. ./test-lib.sh
|
|
|
|
|
|
|
|
test_tick
|
|
|
|
|
|
|
|
test_expect_success setup '
|
|
|
|
echo A >fileA.t &&
|
|
|
|
echo B >fileB.t &&
|
|
|
|
echo C >fileC.t &&
|
|
|
|
echo D >fileD.t &&
|
|
|
|
git add fileA.t fileB.t fileC.t fileD.t &&
|
|
|
|
git commit -m "files" &&
|
|
|
|
|
|
|
|
git tag checkpoint
|
|
|
|
'
|
|
|
|
|
|
|
|
restore_checkpoint () {
|
|
|
|
git reset --hard checkpoint
|
|
|
|
}
|
|
|
|
|
|
|
|
verify_expect () {
|
|
|
|
git status --porcelain --untracked-files=no -- fileA.t fileB.t fileC.t fileD.t >actual &&
|
|
|
|
test_cmp expect actual
|
|
|
|
}
|
|
|
|
|
|
|
|
test_expect_success 'simplest' '
|
|
|
|
restore_checkpoint &&
|
|
|
|
|
|
|
|
cat >expect <<-\EOF &&
|
|
|
|
D fileA.t
|
|
|
|
EOF
|
|
|
|
|
|
|
|
echo fileA.t | git rm --pathspec-from-file=- &&
|
|
|
|
verify_expect
|
|
|
|
'
|
|
|
|
|
|
|
|
test_expect_success '--pathspec-file-nul' '
|
|
|
|
restore_checkpoint &&
|
|
|
|
|
|
|
|
cat >expect <<-\EOF &&
|
|
|
|
D fileA.t
|
|
|
|
D fileB.t
|
|
|
|
EOF
|
|
|
|
|
|
|
|
printf "fileA.t\0fileB.t\0" | git rm --pathspec-from-file=- --pathspec-file-nul &&
|
|
|
|
verify_expect
|
|
|
|
'
|
|
|
|
|
|
|
|
test_expect_success 'only touches what was listed' '
|
|
|
|
restore_checkpoint &&
|
|
|
|
|
|
|
|
cat >expect <<-\EOF &&
|
|
|
|
D fileB.t
|
|
|
|
D fileC.t
|
|
|
|
EOF
|
|
|
|
|
|
|
|
printf "fileB.t\nfileC.t\n" | git rm --pathspec-from-file=- &&
|
|
|
|
verify_expect
|
|
|
|
'
|
|
|
|
|
|
|
|
test_expect_success 'error conditions' '
|
|
|
|
restore_checkpoint &&
|
|
|
|
echo fileA.t >list &&
|
|
|
|
|
|
|
|
test_must_fail git rm --pathspec-from-file=list -- fileA.t 2>err &&
|
|
|
|
test_i18ngrep -e ".--pathspec-from-file. and pathspec arguments cannot be used together" err &&
|
rm: support the --pathspec-from-file option
Decisions taken for simplicity:
1) It is not allowed to pass pathspec in both args and file.
Adjustments were needed for `if (!argc)` block:
This code actually means "pathspec is not present". Previously, pathspec
could only come from commandline arguments, so testing for `argc` was a
valid way of testing for the presence of pathspec. But this is no longer
true with `--pathspec-from-file`.
During the entire `--pathspec-from-file` story, I tried to keep its
behavior very close to giving pathspec on commandline, so that switching
from one to another doesn't involve any surprises.
However, throwing usage at user in the case of empty
`--pathspec-from-file` would puzzle because there's nothing wrong with
"usage" (that is, argc/argv array).
On the other hand, throwing usage in the old case also feels bad to me.
While it's less of a puzzle, I (as user) never liked the experience of
comparing my commandline to "usage", trying to spot a difference. Since
it's already known what the error is, it feels a lot better to give that
specific error to user.
Judging from [1] it doesn't seem that showing usage in this case was
important (the patch was to avoid segfault), and it doesn't fit into how
other commands react to empty pathspec (see for example `git add` with a
custom message).
Therefore, I decided to show new error text in both cases. In order to
continue testing for error early, I moved `parse_pathspec()` higher. Now
it happens before `read_cache()` / `hold_locked_index()` /
`setup_work_tree()`, which shouldn't cause any issues.
[1] Commit 7612a1ef ("git-rm: honor -n flag" 2006-06-09)
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
5 years ago
|
|
|
|
|
|
|
test_must_fail git rm --pathspec-file-nul 2>err &&
|
|
|
|
test_i18ngrep -e "the option .--pathspec-file-nul. requires .--pathspec-from-file." err &&
|
rm: support the --pathspec-from-file option
Decisions taken for simplicity:
1) It is not allowed to pass pathspec in both args and file.
Adjustments were needed for `if (!argc)` block:
This code actually means "pathspec is not present". Previously, pathspec
could only come from commandline arguments, so testing for `argc` was a
valid way of testing for the presence of pathspec. But this is no longer
true with `--pathspec-from-file`.
During the entire `--pathspec-from-file` story, I tried to keep its
behavior very close to giving pathspec on commandline, so that switching
from one to another doesn't involve any surprises.
However, throwing usage at user in the case of empty
`--pathspec-from-file` would puzzle because there's nothing wrong with
"usage" (that is, argc/argv array).
On the other hand, throwing usage in the old case also feels bad to me.
While it's less of a puzzle, I (as user) never liked the experience of
comparing my commandline to "usage", trying to spot a difference. Since
it's already known what the error is, it feels a lot better to give that
specific error to user.
Judging from [1] it doesn't seem that showing usage in this case was
important (the patch was to avoid segfault), and it doesn't fit into how
other commands react to empty pathspec (see for example `git add` with a
custom message).
Therefore, I decided to show new error text in both cases. In order to
continue testing for error early, I moved `parse_pathspec()` higher. Now
it happens before `read_cache()` / `hold_locked_index()` /
`setup_work_tree()`, which shouldn't cause any issues.
[1] Commit 7612a1ef ("git-rm: honor -n flag" 2006-06-09)
Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
5 years ago
|
|
|
|
|
|
|
>empty_list &&
|
|
|
|
test_must_fail git rm --pathspec-from-file=empty_list 2>err &&
|
|
|
|
test_i18ngrep -e "No pathspec was given. Which files should I remove?" err
|
|
|
|
'
|
|
|
|
|
|
|
|
test_done
|