| Message ID | 095aa266c2bfdda47ed722fbc5a0d9c94132fbf1.1642888562.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: In-core git merge-tree ("Server side merges") | expand |
On Sat, Jan 22 2022, Elijah Newren via GitGitGadget wrote: > 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. > + > + /* 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); } > - 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 :)
On Mon, Jan 24, 2022 at 2:00 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > On Sat, Jan 22 2022, Elijah Newren via GitGitGadget wrote: > > > 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. A string_list_grow() would probably be helpful to add at some point; then it could also be used in process_entries(). > > + > > + /* 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); > } How did I not know about and not find for_each_string_list_item() when I was writing this code a couple years ago? (and still didn't learn of it until now?) Thanks for the pointer. Won't change anything right now, though, since... > > - 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 :) yep, so I won't change anything now, but yes th for_each_string_list_item() tip is still useful as a heads up. Thanks.
Hi Elijah, On Sat, 22 Jan 2022, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@gmail.com> > > No functional changes included in this patch; it's just a preparatory > step to allow the printed messages to be handled differently by other > callers, such as in `git merge-tree --write-tree`. Looks good. FWIW I cheated and looked at the output of git show pr-1122/newren/in-core-merge-tree-v1~7 \ --color-moved \ --color-moved-ws=allow-indentation-change (after fetching the tag from https://github.com/gitgitgadget/git) instead of this patch (which is a lot easier to digest for me, because of the color-coding, and since GitGitGadget sent the patch, I know that it is identical to the commit). Ciao, Dscho
diff --git a/merge-ort.c b/merge-ort.c index 9bf15a01db8..f9e35b0f96b 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -4235,6 +4235,45 @@ 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; + + 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); + + /* 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, @@ -4273,42 +4312,8 @@ 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; - - 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); - - /* 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); - } + if (display_update_msgs) + merge_display_update_messages(opt, result); merge_finalize(opt, result); } diff --git a/merge-ort.h b/merge-ort.h index fe599b87868..e5aec45b18f 100644 --- a/merge-ort.h +++ b/merge-ort.h @@ -80,6 +80,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);