Re: [PATCH 05/12] merge-ort: split out a separate display_update_messages() function

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jan 24, 2022, 09:56 UTC
Message-ID: <220124.86czkhihcu.gmgdl@evledraar.gmail.com>
In-Reply-To: <095aa266c2bfdda47ed722fbc5a0d9c94132fbf1.1642888562.git.gitgitgadget@gmail.com>

On Sat, Jan 22 2022, Elijah Newren via GitGitGadget wrote:

Show 5 quoted lines

> From: Elijah Newren <newren@gmail.com>
> [...]
> +	/* Hack to pre-allocate olist to the desired size */
> +	ALLOC_GROW(olist.items, strmap_get_size(&opti->output),
> +		   olist.alloc);

Perhaps just add a string_list_grow()? But I wonder if this is really needed v.s. just using the default growing pattern here.

Show 13 quoted lines

> +
> +	/* Put every entry from output into olist, then sort */
> +	strmap_for_each_entry(&opti->output, &iter, e) {
> +		string_list_append(&olist, e->key)->util = e->value;
> +	}
> +	string_list_sort(&olist);
> +
> +	/* Iterate over the items, printing them */
> +	for (i = 0; i < olist.nr; ++i) {
> +		struct strbuf *sb = olist.items[i].util;
> +
> +		printf("%s", sb->buf);
> +	}

Shorter/nicer:
	
	for_each_string_list_item(item, &olist) {
		struct strbuf *sb = item->util;
	        puts(sb->buf);
	}

Show 29 quoted lines

> -	if (display_update_msgs) {
> -		struct merge_options_internal *opti = result->priv;
> -		struct hashmap_iter iter;
> -		struct strmap_entry *e;
> -		struct string_list olist = STRING_LIST_INIT_NODUP;
> -		int i;
> -
> -		if (opt->record_conflict_msgs_as_headers)
> -			BUG("Either display conflict messages or record them as headers, not both");
> -
> -		trace2_region_enter("merge", "display messages", opt->repo);
> -
> -		/* Hack to pre-allocate olist to the desired size */
> -		ALLOC_GROW(olist.items, strmap_get_size(&opti->output),
> -			   olist.alloc);
> -
> -		/* Put every entry from output into olist, then sort */
> -		strmap_for_each_entry(&opti->output, &iter, e) {
> -			string_list_append(&olist, e->key)->util = e->value;
> -		}
> -		string_list_sort(&olist);
> -
> -		/* Iterate over the items, printing them */
> -		for (i = 0; i < olist.nr; ++i) {
> -			struct strbuf *sb = olist.items[i].util;
> -
> -			printf("%s", sb->buf);
> -		}
> -		string_list_clear(&olist, 0);

Ah, at this point I see you're just moving code around :) Sending this anyway in case it's useful :)