David Aguilar <davvid@xxxxxxxxx> writes:
> This is cleanup refactoring that Junio suggested when
> 5bafb3576a (difftool: fix symlink-file writing in dir-diff mode, 2021-09-22)
> touched this area of the code.
Not really what I would want to take credit for ;-)
> +static void write_entry(const char *path, const char *content,
> + struct strbuf *buf, size_t len)
> +{
> + if (!*content)
> + return;
I am not sure "this function is unable to write an empty file" is a
limitation we want to give to an otherwise more or less generic
helper function that may be useful in this file (it probably is not
very useful outside difftool, as what add_path() does seems quite
specific to it).
Also, is "write entry" a good name for this helper? The fact that
the contents came from entry->$side is lost inside this callee. It
looks more like "create a file for <path> and write <content> to it",
i.e. a variant of write_file() but inside the tree specified by the
extra <buf, len> pair. So perhaps
write_file_in_directory(buf, len, path, content);
to match how the write_file() takes its parameters? While
write_file() takes a single pathname with the payload, this thing
takes three parameters <buf, len, path> to specify to which
"file-in-directory" the payload is written.
> + add_path(buf, len, path);
> + ensure_leading_directories(buf->buf);
> + unlink(buf->buf);
> + write_file(buf->buf, "%s", content);
> +}
Other than these two minor points, this looks good to me.
Thanks.
