* [PATCH 0/2] Fix autocorrect in work tree for bare repository
@ 2022-10-28 15:24 Simon Gerber via GitGitGadget
2022-10-28 15:24 ` [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone Simon Gerber via GitGitGadget
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Simon Gerber via GitGitGadget @ 2022-10-28 15:24 UTC (permalink / raw)
To: git; +Cc: Simon Gerber
Currently, auto correction doesn't work reliably for commands which must run
in a work tree (e.g. git status) in Git work trees which are created from a
bare repository.
This patch series adds a test case illustrating the issue and proposes a fix
which adjusts the usage of read_early_config() in help_unknown_cmd() to
match other usages of read_early_config(). In particular the patch removes
the call to git_default_config() in the read config callback.
Simon Gerber (2):
tests: add test case for autocorrect in work tree for bare clone
help.c: don't call git_default_config in git_unknown_cmd_config
help.c | 2 +-
t/t9005-help-autocorrect-worktree.sh | 29 ++++++++++++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
create mode 100755 t/t9005-help-autocorrect-worktree.sh
base-commit: e7e5c6f715b2de7bea0d39c7d2ba887335b40aa0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1373%2Fsimu%2Ffix%2Fautocorrect-bare-worktree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1373/simu/fix/autocorrect-bare-worktree-v1
Pull-Request: https://github.com/git/git/pull/1373
--
gitgitgadget
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone
2022-10-28 15:24 [PATCH 0/2] Fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
@ 2022-10-28 15:24 ` Simon Gerber via GitGitGadget
2022-10-28 19:28 ` Junio C Hamano
2022-10-28 15:24 ` [PATCH 2/2] help.c: don't call git_default_config in git_unknown_cmd_config Simon Gerber via GitGitGadget
2022-10-29 19:56 ` [PATCH v2] help.c: fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
2 siblings, 1 reply; 9+ messages in thread
From: Simon Gerber via GitGitGadget @ 2022-10-28 15:24 UTC (permalink / raw)
To: git; +Cc: Simon Gerber, Simon Gerber
From: Simon Gerber <gesimu@gmail.com>
Currently, auto correction doesn't work reliably for commands which must
run in a work tree (e.g. `git status`) in Git work trees which are
created from a bare repository.
This commit adds a test which shows the broken autocompletion by calling
`git stauts` which gets autocorrected to `git status`. This works fine
in a work tree created with `git worktree add` from a regular clone, but
doesn't work in a work tree created with `git worktree add` from a bare
clone.
Signed-off-by: Simon Gerber <gesimu@gmail.com>
---
t/t9005-help-autocorrect-worktree.sh | 29 ++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
create mode 100755 t/t9005-help-autocorrect-worktree.sh
diff --git a/t/t9005-help-autocorrect-worktree.sh b/t/t9005-help-autocorrect-worktree.sh
new file mode 100755
index 00000000000..4fecc8a8e01
--- /dev/null
+++ b/t/t9005-help-autocorrect-worktree.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+
+test_description='test autocorrect in work tree based on bare repository'
+. ./test-lib.sh
+
+test_expect_success 'setup non-bare' '
+ echo one >file &&
+ git add file &&
+ git commit -m one &&
+ echo two >file &&
+ git commit -a -m two
+'
+
+test_expect_success 'setup bare' '
+ git clone --bare . bare.git &&
+ cd bare.git
+'
+
+test_expect_success 'setup worktree from bare' '
+ git worktree add ../bare-wt &&
+ cd ../bare-wt
+'
+
+test_expect_success 'autocorrect works in work tree created from bare repo' '
+ git config help.autocorrect immediate &&
+ git stauts
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] help.c: don't call git_default_config in git_unknown_cmd_config
2022-10-28 15:24 [PATCH 0/2] Fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
2022-10-28 15:24 ` [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone Simon Gerber via GitGitGadget
@ 2022-10-28 15:24 ` Simon Gerber via GitGitGadget
2022-10-29 19:56 ` [PATCH v2] help.c: fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
2 siblings, 0 replies; 9+ messages in thread
From: Simon Gerber via GitGitGadget @ 2022-10-28 15:24 UTC (permalink / raw)
To: git; +Cc: Simon Gerber, Simon Gerber
From: Simon Gerber <gesimu@gmail.com>
Currently, auto correction doesn't work reliably for commands which must
run in a work tree (e.g. `git status`) in Git work trees which are
created from a bare repository.
As far as I'm able to determine, this has been broken since commit
659fef199f (help: use early config when autocorrecting aliases,
2017-06-14), where the call to `git_config()` in `help_unknown_cmd()`
was replaced with a call to `read_early_config()`. From what I can tell,
the actual cause for the unexpected error is that we call
`git_default_config()` in the `git_unknown_cmd_config` callback instead
of simply returning `0` for config entries which we aren't interested
in.
Calling `git_default_config()` in this callback to `read_early_config()`
seems like a bad idea since those calls will initialize a bunch of state
in `environment.c` (among other things `is_bare_repository_cfg`) before
we've properly detected that we're running in a work tree.
All other callbacks provided to `read_early_config()` appear to only
extract their configurations while simply returning `0` for all other
config keys.
This commit changes the `git_unknown_cmd_config` callback to not call
`git_default_config()`. Instead we also simply return `0` for config
keys which we're not interested in.
Signed-off-by: Simon Gerber <gesimu@gmail.com>
---
help.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/help.c b/help.c
index d04542d8261..ae534ff0bae 100644
--- a/help.c
+++ b/help.c
@@ -563,7 +563,7 @@ static int git_unknown_cmd_config(const char *var, const char *value, void *cb)
if (skip_prefix(var, "alias.", &p))
add_cmdname(&aliases, p, strlen(p));
- return git_default_config(var, value, cb);
+ return 0;
}
static int levenshtein_compare(const void *p1, const void *p2)
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone
2022-10-28 15:24 ` [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone Simon Gerber via GitGitGadget
@ 2022-10-28 19:28 ` Junio C Hamano
2022-10-29 8:07 ` Simon Gerber
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2022-10-28 19:28 UTC (permalink / raw)
To: Simon Gerber via GitGitGadget; +Cc: git, Simon Gerber
"Simon Gerber via GitGitGadget" <gitgitgadget@gmail.com> writes:
> t/t9005-help-autocorrect-worktree.sh | 29 ++++++++++++++++++++++++++++
Don't we have existing test script for auto-correction? Is it
sensible to waste a new fail for this single test? I doubt it.
Perhaps appending after 9003 a new test instead?
> diff --git a/t/t9005-help-autocorrect-worktree.sh b/t/t9005-help-autocorrect-worktree.sh
> new file mode 100755
> index 00000000000..4fecc8a8e01
> --- /dev/null
> +++ b/t/t9005-help-autocorrect-worktree.sh
> @@ -0,0 +1,29 @@
> +#!/bin/sh
> +
> +test_description='test autocorrect in work tree based on bare repository'
> +. ./test-lib.sh
> +
> +test_expect_success 'setup non-bare' '
> + echo one >file &&
> + git add file &&
> + git commit -m one &&
> + echo two >file &&
> + git commit -a -m two
> +'
Wouldn't "test_commit" be simpler to use than rolling our own here?
If we piggy-back on set-up in an existing test script, there
probably is already a history the single test to attempt running
'git staut' can use. After all, that test does not care an iota
what is in the history.
> +
> +test_expect_success 'setup bare' '
> + git clone --bare . bare.git &&
> + cd bare.git
Do not chdir around in a test script, especially in separate steps.
> +'
> +
> +test_expect_success 'setup worktree from bare' '
> + git worktree add ../bare-wt &&
> + cd ../bare-wt
> +'
Ditto.
Either do it in a subshell in a single step, or
(
git clone --bare . bare.git &&
cd bare.git &&
git worktree add ../worktree &&
cd ../worktree &&
git -c help.autocorrect=immediate staut
)
use "git -C <over-there>" form, e.g.
git clone --bare . bare.git &&
git -C bare.git worktree add ../worktree &&
git -C worktree -c help.autocorrect=immediate staut
> +test_expect_success 'autocorrect works in work tree created from bare repo' '
When patch 1/2 is applied without 2/2, this test_expect_success will
not be satisfied, breaking future bisection.
For a small change like this, have the code change *and* test that
verifies the new behaviour in a single step. That way
* you do not break bisection.
* if somebody wants to cherry-pick the fix to an older maintenance
tracks, they can do so by picking a single unit, fix and
verification combined together.
* acceptance or review can be done by checking the end-result and
then tentatively reverting only the code change with something
like
$ git show -- ':!t/' | git apply -R
and see the test that expects success actually fails without the fix.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone
2022-10-28 19:28 ` Junio C Hamano
@ 2022-10-29 8:07 ` Simon Gerber
0 siblings, 0 replies; 9+ messages in thread
From: Simon Gerber @ 2022-10-29 8:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Simon Gerber via GitGitGadget, git
On Fri, 28 Oct 2022 at 21:28, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Simon Gerber via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > t/t9005-help-autocorrect-worktree.sh | 29 ++++++++++++++++++++++++++++
>
> Don't we have existing test script for auto-correction? Is it
> sensible to waste a new fail for this single test? I doubt it.
>
> Perhaps appending after 9003 a new test instead?
I'll move the test to 9003. I was considering doing so in the
beginning, but was hesitant to add an only vaguely related test in
9003 which has the description "help.autocorrect finding a match".
>
> > diff --git a/t/t9005-help-autocorrect-worktree.sh b/t/t9005-help-autocorrect-worktree.sh
> > new file mode 100755
> > index 00000000000..4fecc8a8e01
> > --- /dev/null
> > +++ b/t/t9005-help-autocorrect-worktree.sh
> > @@ -0,0 +1,29 @@
> > +#!/bin/sh
> > +
> > +test_description='test autocorrect in work tree based on bare repository'
> > +. ./test-lib.sh
> > +
> > +test_expect_success 'setup non-bare' '
> > + echo one >file &&
> > + git add file &&
> > + git commit -m one &&
> > + echo two >file &&
> > + git commit -a -m two
> > +'
>
> Wouldn't "test_commit" be simpler to use than rolling our own here?
> If we piggy-back on set-up in an existing test script, there
> probably is already a history the single test to attempt running
> 'git staut' can use. After all, that test does not care an iota
> what is in the history.
>
> > +
> > +test_expect_success 'setup bare' '
> > + git clone --bare . bare.git &&
> > + cd bare.git
>
> Do not chdir around in a test script, especially in separate steps.
>
> > +'
> > +
> > +test_expect_success 'setup worktree from bare' '
> > + git worktree add ../bare-wt &&
> > + cd ../bare-wt
> > +'
>
> Ditto.
>
> Either do it in a subshell in a single step, or
>
> (
> git clone --bare . bare.git &&
> cd bare.git &&
> git worktree add ../worktree &&
> cd ../worktree &&
> git -c help.autocorrect=immediate staut
> )
>
> use "git -C <over-there>" form, e.g.
>
> git clone --bare . bare.git &&
> git -C bare.git worktree add ../worktree &&
> git -C worktree -c help.autocorrect=immediate staut
>
Thanks for taking the time to point out how I'm supposed to use the
test framework, I was pretty lost trying to write my test and ended up
copying some bits from 7103.
> > +test_expect_success 'autocorrect works in work tree created from bare repo' '
>
> When patch 1/2 is applied without 2/2, this test_expect_success will
> not be satisfied, breaking future bisection.
>
> For a small change like this, have the code change *and* test that
> verifies the new behaviour in a single step. That way
>
> * you do not break bisection.
>
> * if somebody wants to cherry-pick the fix to an older maintenance
> tracks, they can do so by picking a single unit, fix and
> verification combined together.
>
> * acceptance or review can be done by checking the end-result and
> then tentatively reverting only the code change with something
> like
>
> $ git show -- ':!t/' | git apply -R
>
> and see the test that expects success actually fails without the fix.
That makes sense. I'll resubmit this as a single patch.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] help.c: fix autocorrect in work tree for bare repository
2022-10-28 15:24 [PATCH 0/2] Fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
2022-10-28 15:24 ` [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone Simon Gerber via GitGitGadget
2022-10-28 15:24 ` [PATCH 2/2] help.c: don't call git_default_config in git_unknown_cmd_config Simon Gerber via GitGitGadget
@ 2022-10-29 19:56 ` Simon Gerber via GitGitGadget
2022-12-12 16:38 ` Simon Gerber
2022-12-13 1:37 ` Junio C Hamano
2 siblings, 2 replies; 9+ messages in thread
From: Simon Gerber via GitGitGadget @ 2022-10-29 19:56 UTC (permalink / raw)
To: git; +Cc: Simon Gerber, Simon Gerber
From: Simon Gerber <gesimu@gmail.com>
Currently, auto correction doesn't work reliably for commands which must
run in a work tree (e.g. `git status`) in Git work trees which are
created from a bare repository.
As far as I'm able to determine, this has been broken since commit
659fef199f (help: use early config when autocorrecting aliases,
2017-06-14), where the call to `git_config()` in `help_unknown_cmd()`
was replaced with a call to `read_early_config()`. From what I can tell,
the actual cause for the unexpected error is that we call
`git_default_config()` in the `git_unknown_cmd_config` callback instead
of simply returning `0` for config entries which we aren't interested
in.
Calling `git_default_config()` in this callback to `read_early_config()`
seems like a bad idea since those calls will initialize a bunch of state
in `environment.c` (among other things `is_bare_repository_cfg`) before
we've properly detected that we're running in a work tree.
All other callbacks provided to `read_early_config()` appear to only
extract their configurations while simply returning `0` for all other
config keys.
This commit changes the `git_unknown_cmd_config` callback to not call
`git_default_config()`. Instead we also simply return `0` for config
keys which we're not interested in.
Additionally the commit adds a new test case covering `help.autocorrect`
in a work tree created from a bare clone.
Signed-off-by: Simon Gerber <gesimu@gmail.com>
---
Fix autocorrect in work tree for bare repository
Currently, auto correction doesn't work reliably for commands which must
run in a work tree (e.g. git status) in Git work trees which are created
from a bare repository.
This patch adds a test case illustrating the issue and proposes a fix
which adjusts the usage of read_early_config() in help_unknown_cmd() to
match other usages of read_early_config(). In particular the patch
removes the call to git_default_config() in the read config callback.
Changes since v1 (both suggested by Junio):
* Moved test to 9003
* Squashed change and test into a single commit
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1373%2Fsimu%2Ffix%2Fautocorrect-bare-worktree-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1373/simu/fix/autocorrect-bare-worktree-v2
Pull-Request: https://github.com/git/git/pull/1373
Range-diff vs v1:
1: d0a6eea93b5 < -: ----------- tests: add test case for autocorrect in work tree for bare clone
2: 7a2bef32439 ! 1: 4c598dbe778 help.c: don't call git_default_config in git_unknown_cmd_config
@@ Metadata
Author: Simon Gerber <gesimu@gmail.com>
## Commit message ##
- help.c: don't call git_default_config in git_unknown_cmd_config
+ help.c: fix autocorrect in work tree for bare repository
Currently, auto correction doesn't work reliably for commands which must
run in a work tree (e.g. `git status`) in Git work trees which are
@@ Commit message
`git_default_config()`. Instead we also simply return `0` for config
keys which we're not interested in.
+ Additionally the commit adds a new test case covering `help.autocorrect`
+ in a work tree created from a bare clone.
+
Signed-off-by: Simon Gerber <gesimu@gmail.com>
## help.c ##
@@ help.c: static int git_unknown_cmd_config(const char *var, const char *value, vo
}
static int levenshtein_compare(const void *p1, const void *p2)
+
+ ## t/t9003-help-autocorrect.sh ##
+@@ t/t9003-help-autocorrect.sh: test_expect_success 'autocorrect can be declined altogether' '
+ test_line_count = 1 actual
+ '
+
++test_expect_success 'autocorrect works in work tree created from bare repo' '
++ git clone --bare . bare.git &&
++ git -C bare.git worktree add ../worktree &&
++ git -C worktree -c help.autocorrect=immediate stauts
++'
++
+ test_done
help.c | 2 +-
t/t9003-help-autocorrect.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/help.c b/help.c
index d04542d8261..ae534ff0bae 100644
--- a/help.c
+++ b/help.c
@@ -563,7 +563,7 @@ static int git_unknown_cmd_config(const char *var, const char *value, void *cb)
if (skip_prefix(var, "alias.", &p))
add_cmdname(&aliases, p, strlen(p));
- return git_default_config(var, value, cb);
+ return 0;
}
static int levenshtein_compare(const void *p1, const void *p2)
diff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh
index f00deaf3815..f5b6b4f746b 100755
--- a/t/t9003-help-autocorrect.sh
+++ b/t/t9003-help-autocorrect.sh
@@ -60,4 +60,10 @@ test_expect_success 'autocorrect can be declined altogether' '
test_line_count = 1 actual
'
+test_expect_success 'autocorrect works in work tree created from bare repo' '
+ git clone --bare . bare.git &&
+ git -C bare.git worktree add ../worktree &&
+ git -C worktree -c help.autocorrect=immediate stauts
+'
+
test_done
base-commit: e7e5c6f715b2de7bea0d39c7d2ba887335b40aa0
--
gitgitgadget
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] help.c: fix autocorrect in work tree for bare repository
2022-10-29 19:56 ` [PATCH v2] help.c: fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
@ 2022-12-12 16:38 ` Simon Gerber
2022-12-13 1:37 ` Junio C Hamano
1 sibling, 0 replies; 9+ messages in thread
From: Simon Gerber @ 2022-12-12 16:38 UTC (permalink / raw)
To: Simon Gerber via GitGitGadget; +Cc: git, Junio C Hamano
On Sat, 29 Oct 2022 at 21:56, Simon Gerber via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> [commit message omitted]
> ---
> Fix autocorrect in work tree for bare repository
>
> Currently, auto correction doesn't work reliably for commands which must
> run in a work tree (e.g. git status) in Git work trees which are created
> from a bare repository.
>
> This patch adds a test case illustrating the issue and proposes a fix
> which adjusts the usage of read_early_config() in help_unknown_cmd() to
> match other usages of read_early_config(). In particular the patch
> removes the call to git_default_config() in the read config callback.
>
> Changes since v1 (both suggested by Junio):
>
> * Moved test to 9003
> * Squashed change and test into a single commit
>
> [diff omitted]
Hi all,
Just wanted to check what the state on this patch is (side-note: I
just noticed that I didn't cc Junio in the v2 submission, my
apologies). As described in the quote (and the parent email), the
patch fixes how auto correct initializes the config. Version 2
addresses the points raised on v1, in particular, I've moved the test
to 9003, and have squashed the change into a single commit. Please let
me know if further changes are necessary.
-- Simon
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] help.c: fix autocorrect in work tree for bare repository
2022-10-29 19:56 ` [PATCH v2] help.c: fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
2022-12-12 16:38 ` Simon Gerber
@ 2022-12-13 1:37 ` Junio C Hamano
2022-12-13 9:48 ` Simon Gerber
1 sibling, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2022-12-13 1:37 UTC (permalink / raw)
To: Simon Gerber via GitGitGadget; +Cc: git, Simon Gerber
"Simon Gerber via GitGitGadget" <gitgitgadget@gmail.com> writes:
> Calling `git_default_config()` in this callback to `read_early_config()`
> seems like a bad idea since those calls will initialize a bunch of state
> in `environment.c` (among other things `is_bare_repository_cfg`) before
> we've properly detected that we're running in a work tree.
>
> All other callbacks provided to `read_early_config()` appear to only
> extract their configurations while simply returning `0` for all other
> config keys.
Very good observation and justification for this change.
> help.c | 2 +-
> t/t9003-help-autocorrect.sh | 6 ++++++
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/help.c b/help.c
> index d04542d8261..ae534ff0bae 100644
> --- a/help.c
> +++ b/help.c
> @@ -563,7 +563,7 @@ static int git_unknown_cmd_config(const char *var, const char *value, void *cb)
> if (skip_prefix(var, "alias.", &p))
> add_cmdname(&aliases, p, strlen(p));
>
> - return git_default_config(var, value, cb);
> + return 0;
> }
And this is exactly what the proposed log message promises to do.
> static int levenshtein_compare(const void *p1, const void *p2)
> diff --git a/t/t9003-help-autocorrect.sh b/t/t9003-help-autocorrect.sh
> index f00deaf3815..f5b6b4f746b 100755
> --- a/t/t9003-help-autocorrect.sh
> +++ b/t/t9003-help-autocorrect.sh
> @@ -60,4 +60,10 @@ test_expect_success 'autocorrect can be declined altogether' '
> test_line_count = 1 actual
> '
>
> +test_expect_success 'autocorrect works in work tree created from bare repo' '
> + git clone --bare . bare.git &&
> + git -C bare.git worktree add ../worktree &&
> + git -C worktree -c help.autocorrect=immediate stauts
The reason why this third line is a sufficient test is...?
If "status" is invoked successfully, it would not exit with non-zero
status as long as it correctly notices that it was invoked in a
worktree (as opposed to the current code without your fix, which
would say "nah, where you are running there is no worktree", that is
incorrect), but one scenario I am a bit worried about is what if the
tester has an entry on $PATH that has "git-static" or whatever that
is similar enough to "status", to cause autocorrect work differently
from the case where "git status" would be the only plausible case.
But then we can tell such a tester "don't do that, then" ;-)
Let's queue the patch as-is and see what others think.
Thanks.
> +'
> +
> test_done
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] help.c: fix autocorrect in work tree for bare repository
2022-12-13 1:37 ` Junio C Hamano
@ 2022-12-13 9:48 ` Simon Gerber
0 siblings, 0 replies; 9+ messages in thread
From: Simon Gerber @ 2022-12-13 9:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Simon Gerber via GitGitGadget, git
On Tue, 13 Dec 2022 at 02:37, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Simon Gerber via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > --- a/t/t9003-help-autocorrect.sh
> > +++ b/t/t9003-help-autocorrect.sh
> > @@ -60,4 +60,10 @@ test_expect_success 'autocorrect can be declined altogether' '
> > test_line_count = 1 actual
> > '
> >
> > +test_expect_success 'autocorrect works in work tree created from bare repo' '
> > + git clone --bare . bare.git &&
> > + git -C bare.git worktree add ../worktree &&
> > + git -C worktree -c help.autocorrect=immediate stauts
>
> The reason why this third line is a sufficient test is...?
>
> If "status" is invoked successfully, it would not exit with non-zero
> status as long as it correctly notices that it was invoked in a
> worktree (as opposed to the current code without your fix, which
> would say "nah, where you are running there is no worktree", that is
> incorrect), but one scenario I am a bit worried about is what if the
> tester has an entry on $PATH that has "git-static" or whatever that
> is similar enough to "status", to cause autocorrect work differently
> from the case where "git status" would be the only plausible case.
Thanks for pointing out that autocorrect could work differently for a
tester depending on what's in their path. I didn't consider that case
at all.
>
> But then we can tell such a tester "don't do that, then" ;-)
I think that's fine in this case.
>
> Let's queue the patch as-is and see what others think.
Awesome, thanks
>
> Thanks.
>
> > +'
> > +
> > test_done
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-12-13 9:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-28 15:24 [PATCH 0/2] Fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
2022-10-28 15:24 ` [PATCH 1/2] tests: add test case for autocorrect in work tree for bare clone Simon Gerber via GitGitGadget
2022-10-28 19:28 ` Junio C Hamano
2022-10-29 8:07 ` Simon Gerber
2022-10-28 15:24 ` [PATCH 2/2] help.c: don't call git_default_config in git_unknown_cmd_config Simon Gerber via GitGitGadget
2022-10-29 19:56 ` [PATCH v2] help.c: fix autocorrect in work tree for bare repository Simon Gerber via GitGitGadget
2022-12-12 16:38 ` Simon Gerber
2022-12-13 1:37 ` Junio C Hamano
2022-12-13 9:48 ` Simon Gerber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).