On Mon, Jan 24, 2022 at 2:00 AM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> wrote:
>
> On Sat, Jan 22 2022, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@xxxxxxxxx>
> > [...]
> > + /* 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.
