Re: [PATCH v2 05/12] builtin/show-ref: refactor `--exclude-existing` options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 26, 2023 at 11:56:37AM +0200, Patrick Steinhardt wrote:
> It's not immediately obvious options which options are applicable to
> what subcommand in git-show-ref(1) because all options exist as global
> state. This can easily cause confusion for the reader.
>
> Refactor options for the `--exclude-existing` subcommand to be contained
> in a separate structure. This structure is stored on the stack and
> passed down as required. Consequently, it clearly delimits the scope of
> those options and requires the reader to worry less about global state.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>

All makes sense, but...

> @@ -19,8 +19,7 @@ static const char * const show_ref_usage[] = {
>  };
>
>  static int deref_tags, show_head, tags_only, heads_only, found_match, verify,
> -	   quiet, hash_only, abbrev, exclude_arg;
> -static const char *exclude_existing_arg;
> +	   quiet, hash_only, abbrev;
>
>  static void show_one(const char *refname, const struct object_id *oid)
>  {
> @@ -95,6 +94,11 @@ static int add_existing(const char *refname,
>  	return 0;
>  }
>
> +struct exclude_existing_options {
> +	int enabled;

...do we need an .enabled here? I think checking whether or not .pattern
is NULL is sufficient, but perhaps there is another use of .enabled
later on in the series...

Thanks,
Taylor




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux