Re: [PATCH v2 2/4] builtin/multi-pack-index.c: don't handle 'progress' separately

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 15, 2021, 21:39 UTC
Message-ID: <87a6s5au4b.fsf@evledraar.gmail.com>
In-Reply-To: <abcc7fb7312b349562fe6d13e2250496e107c9ed.1613422804.git.me@ttaylorr.com>

On Mon, Feb 15 2021, Taylor Blau wrote:

Show 15 quoted lines

> @@ -31,15 +30,14 @@ int cmd_multi_pack_index(int argc, const char **argv,
>  
>  	git_config(git_default_config, NULL);
>  
> -	opts.progress = isatty(2);
> +	if (isatty(2))
> +		opts.flags |= MIDX_PROGRESS;
>  	argc = parse_options(argc, argv, prefix,
>  			     builtin_multi_pack_index_options,
>  			     builtin_multi_pack_index_usage, 0);
>  
>  	if (!opts.object_dir)
>  		opts.object_dir = get_object_directory();
> -	if (opts.progress)
> -		opts.flags |= MIDX_PROGRESS;

Funnily enough we could also just do:

    opts.flags = isatty(2);

Since there's a grand total of one flag it knows about, and MIDX_PROGRESS is defined as 1.

Not the problem of this series really, just a nit: In efbc3aee08d (midx: add MIDX_PROGRESS flag, 2019-10-21) we added this flag, and around the same time the similar commit-graph code got refactored to have an enum of flags in 5af80394521 (commit-graph: collapse parameters into flags, 2019-06-12).

I prefer the commit-graph way of having a clean boundary between the two a bit more, and then just setting a flag based on an OPT_BOOL...