Re: [PATCH v2 05/12] builtin/show-ref: refactor `--exclude-existing` options
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Oct 30, 2023, 18:37 UTC
- Message-ID
- <ZT/32jI62GQKPlcp@nand.local>
- In-Reply-To
- <bed2a8a07696371e07c0b2d1282ed51c0b1b9fee.1698314128.git.ps@pks.im>
On Thu, Oct 26, 2023 at 11:56:37AM +0200, Patrick Steinhardt wrote:
Show 10 quoted lines
> 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@pks.im>
All makes sense, but...
Show 16 quoted lines
> @@ -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