On Wed, May 11, 2016 at 6:07 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>
> This changes semantics, doesn't it? prefix_filename() seems to do a
> lot more than just strbuf_vadd("%s%s", prefix, filename); would do.
>
> It may be a good change (e.g. turn '\' into '/' on Windows), but
> this is way more than "simplify prefixing". It is something else
> whose effect needs to be explained.
On Wed, May 11, 2016 at 6:07 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>
> This changes semantics, doesn't it? prefix_filename() seems to do a
> lot more than just strbuf_vadd("%s%s", prefix, filename); would do.
>
> It may be a good change (e.g. turn '\' into '/' on Windows), but
> this is way more than "simplify prefixing". It is something else
> whose effect needs to be explained.
I admit I forgot about Windows part in prefix_filename(). For
non-Windows code, it's exactly the same behavior. Maybe we should do
this to emphasize that prefix_filename() is no-op when pfx_len is
zero? The same pattern is used elsewhere too (or I'm spreading it in
my local tree..) Unless of course if convert_slashes() is a good thing
to always do, then I need to update my commit message (+Johannes for
this question).
diff --git a/abspath.c b/abspath.c
index 2825de8..bf454e0 100644
--- a/abspath.c
+++ b/abspath.c
@@ -160,8 +160,11 @@ const char *absolute_path(const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static struct strbuf path = STRBUF_INIT;
+
+ if (!pfx_len)
+ return arg;
#ifndef GIT_WINDOWS_NATIVE
- if (!pfx_len || is_absolute_path(arg))
+ if (is_absolute_path(arg))
return arg;
strbuf_reset(&path);
strbuf_add(&path, pfx, pfx_len);
>
>> builtin/worktree.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> index b53f802..f9dac37 100644
>> --- a/builtin/worktree.c
>> +++ b/builtin/worktree.c
>> @@ -337,7 +337,7 @@ static int add(int ac, const char **av, const char *prefix)
>> if (ac < 1 || ac > 2)
>> usage_with_options(worktree_usage, options);
>>
>> - path = prefix ? prefix_filename(prefix, strlen(prefix), av[0]) : av[0];
>> + path = prefix_filename(prefix, strlen(prefix), av[0]);
>> branch = ac < 2 ? "HEAD" : av[1];
>>
>> opts.force_new_branch = !!new_branch_force;
>> @@ -467,6 +467,8 @@ int cmd_worktree(int ac, const char **av, const char *prefix)
>>
>> if (ac < 2)
>> usage_with_options(worktree_usage, options);
>> + if (!prefix)
>> + prefix = "";
>> if (!strcmp(av[1], "add"))
>> return add(ac - 1, av + 1, prefix);
>> if (!strcmp(av[1], "prune"))
--
Duy
--
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