On Wed, Nov 11, 2020 at 8:09 AM Derrick Stolee <stolee@xxxxxxxxx> wrote:
>
> On 11/2/2020 3:43 PM, Elijah Newren wrote:
> > We want to handle paths below a directory before needing to handle the
> > directory itself. Also, we want to handle the directory immediately
> > after the paths below it, so we can't use simple lexicographic ordering
> > from strcmp (which would insert foo.txt between foo and foo/file.c).
> > Copy string_list_df_name_compare() from merge-recursive.c, and set up a
> > string list of paths sorted by that function so that we can iterate in
> > the desired order.
>
> This is at least the second time we've copied something from
> merge-recursive.c. Should we be starting a merge-utils.[c|h] to group
> these together under a common implementation?
I'm actually going to replace the function later for performance
reasons, but trying to make the series as simple as possible prompted
me to "just copy something for a starting point".
There will be more functions that I copy, yes, but since I sometimes
also tweak and since the goal is to delete merge-recursive.[ch], I
didn't really want to set up an infrastructure to share stuff.
> > + /* Put every entry from paths into plist, then sort */
> > strmap_for_each_entry(&opt->priv->paths, &iter, e) {
> > + string_list_append(&plist, e->key)->util = e->value;
> > + }
>
> nit: are braces required here?
It might not be with the current macro definition of
strmap_for_each_entry(), but I think at one point it was (the macro
has undergone some changes over time). Given the difficulty of
digging through the layers of macros (and the possible risk of it
changing in the future with hashmap or strmap changes), I wonder if
it's simpler for readers to just keep them?
