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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 26 Jul 2022 17:04:31 +0200

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.

> +	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...

> +		/*
> +		 * 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.