On 2/23/2021 6:44 PM, Elijah Newren via GitGitGadget wrote:> +static char *get_dirname(const char *filename)
> +{
> + char *slash = strrchr(filename, '/');
> + return slash ? xstrndup(filename, slash-filename) : xstrdup("");
My brain interpreted "slash-filename" as a single token on first
read, which confused me briefly. Inserting spaces would help
readers like me.
> + * (4) Check if applying that directory rename to the original file
> + * would result in a destination filename that is in the
> + * potential rename set. If so, return the index of the
> + * destination file (the index within rename_dst).
> + * This function, idx_possible_rename(), is only responsible for (4).
This helps isolate the important step to care about for the implementation,
while the rest of the context is important, too.
> + char *old_dir, *new_dir, *new_path;
> + int idx;
> +
> + if (!info->setup)
> + return -1;
> +
> + old_dir = get_dirname(filename);
> + new_dir = strmap_get(&info->dir_rename_guess, old_dir);
> + free(old_dir);
> + if (!new_dir)
> + return -1;
> +
> + new_path = xstrfmt("%s/%s", new_dir, get_basename(filename));
This is running in a loop, so `xstrfmt()` might be overkill compared
to something like
strbuf_addstr(&new_path, new_dir);
strbuf_addch(&new_path, '/');
strbuf_addstr(&new_path, get_basename(filename));
but maybe the difference is too small to notice. (notice the type
change to "struct strbuf new_path = STRBUF_INIT;")
> +
> + idx = strintmap_get(&info->idx_map, new_path);
> + free(new_path);
> + return idx;
> +}
Does what it says it does.
Thanks,
-Stolee
