Re: [PATCH 2/3] log: add default decoration filter

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 26, 2022, 15:04 UTC
Message-ID: <220726.86pmhrnc4x.gmgdl@evledraar.gmail.com>
In-Reply-To: <6b40b84773c70244bb13204ec566b713f1bdf865.1658844250.git.gitgitgadget@gmail.com>

On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:

> +static void set_default_decoration_filter(struct decoration_filter *decoration_filter)
> +{
> +	const struct string_list *config_exclude =
> +		repo_config_get_value_multi(the_repository, "log.excludeDecoration");

Just use git_config_get_value_multi. It supplies the_repository.

Show 6 quoted lines

> +	if (config_exclude) {
> +		struct string_list_item *item;
> +		for_each_string_list_item(item, config_exclude)
> +			string_list_append(decoration_filter->exclude_ref_config_pattern,
> +					   item->string);
> +	}

mm, not the first time I've wanted the equivalent of strbuf_addbuf but for a string_list...

> +
> +	if (!decoration_filter->exclude_ref_pattern->nr &&
> +	    !decoration_filter->include_ref_pattern->nr &&
> +	    !decoration_filter->exclude_ref_config_pattern->nr) {

Could return if this isn't true, and save ourselves the subsequent indentation...

Show 18 quoted lines

> +		/*
> +		 * No command-line or config options were given, so
> +		 * populate with sensible defaults.
> +		 */
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/heads/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/notes/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/stash/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/tags/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "refs/remotes/");
> +		string_list_append(decoration_filter->include_ref_pattern,
> +				   "HEAD");
> +	}
> +}

Nit: This would be much easier to read if early on there was a:

	struct string_list *list = decoration_filter->include_ref_pattern;

You'd also be able to avoid much of the line wrapping.