On Mon, 17 Dec 2018 at 22:56, Jonathan Nieder <[email protected]> wrote:
> That makes experimenting with the stripspace command unnecessarily
> fussy. Fix it by discovering the git directory gently, as intended
> all along.
> if (mode == STRIP_COMMENTS || mode == COMMENT_LINES) {
> - setup_git_directory_gently(NULL);
> + setup_git_directory_gently(&nongit);
> git_config(git_default_config, NULL);
> }
Makes me wonder if `setup_git_directory_gently()` should BUG if it
receives NULL. That would require some reshuffling in setup.c, since
`setup_git_directory()` calls out to it with NULL... Just thinking out
loud. In any case, and more importantly, this is the last user providing
NULL except for the one in `setup_git_directory()`.
> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh
> index 5ce47e8af5..0c24a0f9a3 100755
> --- a/t/t0030-stripspace.sh
> +++ b/t/t0030-stripspace.sh
> @@ -430,9 +430,15 @@ test_expect_success '-c with changed comment char' '
> test_expect_success '-c with comment char defined in .git/config' '
> test_config core.commentchar = &&
> printf "= foo\n" >expect &&
> - printf "foo" | (
> - mkdir sub && cd sub && git stripspace -c
> - ) >actual &&
> + rm -fr sub &&
> + mkdir sub &&
> + printf "foo" | git -C sub stripspace -c >actual &&
> + test_cmp expect actual
> +'
A small while-at-it conversion from subshell (with a funny pipe into it)
to `-C sub`. The `rm -fr` invocation is not in the original, so
shouldn't be needed. This one looks safe enough, though it makes me
wonder why you need it. `mkdir -p sub`? Probably not worth a v2.
> +
> +test_expect_success '-c outside git repository' '
> + printf "# foo\n" >expect &&
> + printf "foo" | nongit git stripspace -c >actual &&
> test_cmp expect actual
> '
Nice.
Martin
