| Message ID | 20210505145204.51614-1-oystwa@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e822750e3a68fcaa2ee4a756a3e48c77a691d5b8 |
| Headers | show |
| Series | add: die if both --dry-run and --interactive are given | expand |
Hi, Øystein Walle, Øystein Walle <oystwa@gmail.com> 于2021年5月5日周三 下午10:53写道： > > The interactive machinery does not obey --dry-run. Die appropriate if > both flags are passed. > > Signed-off-by: Øystein Walle <oystwa@gmail.com> > --- > I think a better solution would be to allow this and improve the > interactive machinery to handle --dry-run. This is what I could muster > up at the moment. > > builtin/add.c | 2 ++ > t/t3700-add.sh | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/builtin/add.c b/builtin/add.c > index ea762a41e3..6077eb189f 100644 > --- a/builtin/add.c > +++ b/builtin/add.c > @@ -457,6 +457,8 @@ int cmd_add(int argc, const char **argv, const char *prefix) > if (patch_interactive) > add_interactive = 1; > if (add_interactive) { > + if (show_only) > + die(_("--dry-run is incompatible with --interactive/--patch")); > if (pathspec_from_file) > die(_("--pathspec-from-file is incompatible with --interactive/--patch")); > exit(interactive_add(argv + 1, prefix, patch_interactive)); > diff --git a/t/t3700-add.sh b/t/t3700-add.sh > index b3b122ff97..171b323f50 100755 > --- a/t/t3700-add.sh > +++ b/t/t3700-add.sh > @@ -343,6 +343,10 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out > test_cmp expect.err actual.err > ' > > +test_expect_success 'git add --dry-run --interactive should fail' ' > + test_must_fail git add --dry-run --interactive > +' > + > test_expect_success 'git add empty string should fail' ' > test_must_fail git add "" > ' > -- > 2.27.0 > Thank you for solving this confusion of mine. For the time being, it is an easier solution to make the two flags mutually exclusive. But if we need to make `git add -i` support `--dry-run` in the future, maybe we should passed the parameter `DRY_RUN` (maybe an environment variable) to the sub-process `git -add--interactive.perl` and finally this feature may be implemented in `git apply`. Reported-by: ZheNing Hu <adlternative@gmail.com> Thanks! -- ZheNing Hu
Hi, Junio and thanks for accepting the patch. > The interactive machinery does not obey --dry-run. Die appropriate if > both flags are passed. I just noticed a minor spelling error here: "appropriate" should be "appropriately". I can send a v2 if that's easier for you. Øsse
Øystein Walle <oystwa@gmail.com> writes: > Hi, Junio and thanks for accepting the patch. > >> The interactive machinery does not obey --dry-run. Die appropriate if >> both flags are passed. > > I just noticed a minor spelling error here: "appropriate" should be > "appropriately". I can send a v2 if that's easier for you. Thanks, will locally amend---no need to resend. Thanks for contributing.
diff --git a/builtin/add.c b/builtin/add.c index ea762a41e3..6077eb189f 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -457,6 +457,8 @@ int cmd_add(int argc, const char **argv, const char *prefix) if (patch_interactive) add_interactive = 1; if (add_interactive) { + if (show_only) + die(_("--dry-run is incompatible with --interactive/--patch")); if (pathspec_from_file) die(_("--pathspec-from-file is incompatible with --interactive/--patch")); exit(interactive_add(argv + 1, prefix, patch_interactive)); diff --git a/t/t3700-add.sh b/t/t3700-add.sh index b3b122ff97..171b323f50 100755 --- a/t/t3700-add.sh +++ b/t/t3700-add.sh @@ -343,6 +343,10 @@ test_expect_success 'git add --dry-run --ignore-missing of non-existing file out test_cmp expect.err actual.err ' +test_expect_success 'git add --dry-run --interactive should fail' ' + test_must_fail git add --dry-run --interactive +' + test_expect_success 'git add empty string should fail' ' test_must_fail git add "" '
The interactive machinery does not obey --dry-run. Die appropriate if both flags are passed. Signed-off-by: Øystein Walle <oystwa@gmail.com> --- I think a better solution would be to allow this and improve the interactive machinery to handle --dry-run. This is what I could muster up at the moment. builtin/add.c | 2 ++ t/t3700-add.sh | 4 ++++ 2 files changed, 6 insertions(+)