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

[Fabian Stelzer <fs@xxxxxxxxxxxx>]

On 03.01.2022 13:15, Fabian Stelzer wrote:
> On 31.12.2021 05:04, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@xxxxxxxxx>
> >
> > No functional changes included in this patch; it's just a preparatory
> > step in anticipation of wanting to handle the printed messages
> > differently in `git merge-tree --real`.
>
> Not quite. You are missing:
> +       if (display_update_msgs)
> +               merge_display_update_messages(opt, result, stdout);
>
> in merge_switch_to_result(), which you added in the next commit. Not 
> really relevant for the series as a whole but this commit alone breaks 
> some functionality.

Sorry, just saw this was already noted on the next patch.

> > Signed-off-by: Elijah Newren <newren@xxxxxxxxx>
> > ---
> > merge-ort.c | 69 ++++++++++++++++++++++++++++-------------------------
> > merge-ort.h |  8 +++++++
> > 2 files changed, 44 insertions(+), 33 deletions(-)
> >
> > diff --git a/merge-ort.c b/merge-ort.c
> > index 0342f104836..6237e2fb7fe 100644
> > --- a/merge-ort.c
> > +++ b/merge-ort.c
> > @@ -4197,6 +4197,42 @@ static int record_conflicted_index_entries(struct merge_options *opt)
> > 	return errs;
> > }
> >
> > +void merge_display_update_messages(struct merge_options *opt,
> > +				   struct merge_result *result)
> > +{
> > +	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;
> > +
> > +	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);
> > +
> > +	/* Also include needed rename limit adjustment now */
> > +	diff_warn_rename_limit("merge.renamelimit",
> > +			       opti->renames.needed_limit, 0);
> > +
> > +	trace2_region_leave("merge", "display messages", opt->repo);
> > +}
> > +
> > void merge_switch_to_result(struct merge_options *opt,
> > 			    struct tree *head,
> > 			    struct merge_result *result,
> > @@ -4235,39 +4271,6 @@ void merge_switch_to_result(struct merge_options *opt,
> > 		trace2_region_leave("merge", "write_auto_merge", opt->repo);
> > 	}
> >
> > -	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;
> > -
> > -		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);
> > -
> > -		/* Also include needed rename limit adjustment now */
> > -		diff_warn_rename_limit("merge.renamelimit",
> > -				       opti->renames.needed_limit, 0);
> > -
> > -		trace2_region_leave("merge", "display messages", opt->repo);
> > -	}
> >
> > 	merge_finalize(opt, result);
> > }
> > diff --git a/merge-ort.h b/merge-ort.h
> > index c011864ffeb..1b93555a60b 100644
> > --- a/merge-ort.h
> > +++ b/merge-ort.h
> > @@ -70,6 +70,14 @@ void merge_switch_to_result(struct merge_options *opt,
> > 			    int update_worktree_and_index,
> > 			    int display_update_msgs);
> >
> > +/*
> > + * Display messages about conflicts and which files were 3-way merged.
> > + * Automatically called by merge_switch_to_result() with stream == stdout,
> > + * so only call this when bypassing merge_switch_to_result().
> > + */
> > +void merge_display_update_messages(struct merge_options *opt,
> > +				   struct merge_result *result);
> > +
> > /* Do needed cleanup when not calling merge_switch_to_result() */
> > void merge_finalize(struct merge_options *opt,
> > 		    struct merge_result *result);
> > --
> > gitgitgadget