Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
> quote_path_relative() resetting output buffer is sometimes unnecessary
> as the buffer has never been used, and some other times makes it
> harder for the caller to use (see builtin/grep.c, the caller has to
> insert a string after quote_path_relative)
>
> Move the buffer reset back to call sites when necessary.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> The answer for Jeff's XXX in his patch, why strbuf_insert() instead
> of just adding in advance.
This sounds a lot stronger than "let" to me. All existing callers
that assumed that buf to be emptied by the function now has to clear
it. "quote: stop resetting output buffer of quote_path_relative()"
may better describe what this really does.
How should this interact with the logic in the called function that
used to say "if we ended up returning an empty string because the
path is the same as the base, we should give ./ back", and what
should the return value of this function be?
To answer these questions, you must define the meaning of the string
in the output buffer that already exists when the function is
called. If the caller did this:
strbuf_addstr(&out, "The path relative to your HOME is: ");
quote_path_relative(path, pathlen, &out, "/home/pclouds/");
then the answers are "We still need to add ./ but !out->len is no
longer a good test to decide" and "It should point at the first byte
of what we added, not out->buf".
But if the caller did this instead:
srcdir = "/src/";
strbuf_addstr(&dst, "/dst/");
quote_path_relative(path, pathlen, &dst, srcdir);
printf("cp '%s' '%s'\n", path, dst->buf);
then it is nonsensical to add "./" when out->len is not zero when
the function returns.
So what does it mean to have an existing string in the output buffer
when calling the function? Is it supposed to be a path to a
directory, or just a general uninterpreted string (e.g. a message)?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html