Web lists-archives.com

Re: [PATCH] stripspace: allow -s/-c outside git repository




On Mon, 17 Dec 2018 at 22:56, Jonathan Nieder <jrnieder@xxxxxxxxx> 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