On Mon, Jan 18, 2021 at 1:00 PM Taylor Blau <me@xxxxxxxxxxxx> wrote:
>
> On Thu, Jan 07, 2021 at 09:35:56PM +0000, Elijah Newren via GitGitGadget wrote:
> > From: Elijah Newren <newren@xxxxxxxxx>
> >
> > This is modelled on the version of handle_directory_level_conflicts()
> > from merge-recursive.c, but is massively simplified due to the following
> > factors:
> > * strmap API provides simplifications over using direct hashamp
Whoops, should be "hashmap"
> > * we have a dirs_removed field in struct rename_info that we have an
> > easy way to populate from collect_merge_info(); this was already
> > used in compute_rename_counts() and thus we do not need to check
> > for condition #2.
> > * The removal of condition #2 by handling it earlier in the code also
> > obviates the need to check for condition #3 -- if both sides renamed
> > a directory, meaning that the directory no longer exists on either
> > side, then neither side could have added any new files to that
> > directory, and thus there are no files whose locations we need to
> > move due to such a directory rename.
> >
> > In fact, the same logic that makes condition #3 irrelevant means
> > condition #1 is also irrelevant so we could drop this function.
> > However, it is cheap to check if both sides rename the same directory,
> > and doing so can save future computation. So, simply remove any
> > directories that both sides renamed from the list of directory renames.
>
> Beautiful.
>
> > static void handle_directory_level_conflicts(struct merge_options *opt)
> > {
> > - die("Not yet implemented!");
> > + struct hashmap_iter iter;
> > + struct strmap_entry *entry;
> > + struct string_list duplicated = STRING_LIST_INIT_NODUP;
> > + struct strmap *side1_dir_renames = &opt->priv->renames.dir_renames[1];
> > + struct strmap *side2_dir_renames = &opt->priv->renames.dir_renames[2];
>
> Obviously saying "1" or "MERGE_SIDE1" are two ways of saying the same
> thing, but perhaps the latter is more easily grep-able? Dunno.
>
> > + int i;
> > +
> > + strmap_for_each_entry(side1_dir_renames, &iter, entry) {
> > + if (strmap_contains(side2_dir_renames, entry->key))
> > + string_list_append(&duplicated, entry->key);
> > + }
> > +
> > + for (i=0; i<duplicated.nr; ++i) {
>
> One small nit: this spacing and prefixed ++ reads a little oddly to me.
> Perhaps:
>
> for (i = 0; i < duplicated.nr; i++) {
>
> ?
Sure, will fix.
>
> > + strmap_remove(side1_dir_renames, duplicated.items[i].string, 0);
> > + strmap_remove(side2_dir_renames, duplicated.items[i].string, 0);
> > + }
> > + string_list_clear(&duplicated, 0);
>
> And this looks like a faithful implementation of what you described
> above. Thanks.
>
> Thanks,
> Taylor
