Re: [RFC PATCH] Make rev-parse -q and --is-* quiet

From: Abhishek Kumar <abhishekkumar8222@gmail.com>
Date: Mar 13, 2020, 17:30 UTC
Message-ID: <CAHk66ftWBYF3d_L0-__BP5mKNxBioj57y44yhyqGrtK3TZTjSg@mail.gmail.com>

Show 23 quoted lines

> If rev-parse is called with both -q and an --is-* option, the result is
> provided as the return code of the command, iso. printed to stdout.
>
> This simplifies using these queries in shell scripts:
> git rev-parse --is-bare-repository && do_stuff
> git rev-parse --is-shallow-repository && do_stuff
>
> Signed-off-by: Erlend E. Aasland <erlend.aasland@innova.no>
> ---
> builtin/rev-parse.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 7a00da8203..5a8b404ec7 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -874,24 +874,31 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>                 continue;
>             }
>             if (!strcmp(arg, "--is-inside-git-dir")) {
> -                printf("%s\n", is_inside_git_dir() ? "true"
> -                        : "false");
> +                int is_set = is_inside_git_dir();

Avoid declaration after statement. Move is_set to the beginning of cmd_rev_parse().

Also, be more specific than "is_set". A variable like "is_inside" would be more appropriate.

> +                if (quiet)
> +                    return is_set ? 0 : 1;

Returning prematurely might be a bad idea. If there are more options after "--is-inside-git-dir", they will be not executed. You can maybe rewrite this as:

```
             if (!strcmp(arg, "--is-inside-git-dir")) {
                is_set = is_inside_git_dir();
                if (!quiet)
                        printf("%s\n", is_set ? "true"
                            : "false");
                 continue;
             }
```
And then return the value at the end of function, replacing '0' with !is_set.

Same comment for other blocks.

Show 31 quoted lines

>             if (!strcmp(arg, "--is-inside-work-tree")) {
> -                printf("%s\n", is_inside_work_tree() ? "true"
> -                        : "false");
> +                int is_set = is_inside_work_tree();
> +                if (quiet)
> +                    return is_set ? 0 : 1;
> +                printf("%s\n", is_set ? "true" : "false");
>                 continue;
>             }
>             if (!strcmp(arg, "--is-bare-repository")) {
> -                printf("%s\n", is_bare_repository() ? "true"
> -                        : "false");
> +                int is_set = is_bare_repository();
> +                if (quiet)
> +                    return is_set ? 0 : 1;
> +                printf("%s\n", is_set ? "true" : "false");
>                 continue;
>             }
>             if (!strcmp(arg, "--is-shallow-repository")) {
> -                printf("%s\n",
> -                        is_repository_shallow(the_repository) ? "true"
> -                        : "false");
> +                int is_set = is_repository_shallow(the_repository);
> +                if (quiet)
> +                    return is_set ? 0 : 1;
> +                printf("%s\n", is_set ? "true" : "false");
>                 continue;
>            }
>            if (!strcmp(arg, "--shared-index-path")) {
> --
> 2.25.1

I am wondering if we should stop the script from running if both quiet and --is-* options are passed.

Regards Abhishek