Re: [PATCH v2 6/6] worktree: simplify prefixing paths

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, May 22, 2016 at 5:33 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
> This also makes slash conversion always happen on Windows (a side effect
> of prefix_filename). Which is a good thing.

Selling this patch as a mere simplification seems misguided
(especially as it's subjective whether this is indeed simpler). I'd
find it more easy to buy this change if it was described primarily as
benefiting Windows, in the same vein as[1].

More below...

[1]: http://git.661346.n2.nabble.com/PATCH-Do-not-skip-prefix-filename-when-there-is-no-prefix-tp7657038.html

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> diff --git 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 = "";

Considering that all the other existing git-worktree subcommands
already handle NULL prefix acceptably, it makes me a bit uncomfortable
that this "fix", which could be local to add(), leaks into all the
other subcommands, thus making the assumption that they (or other new
subcommands) will never care about the distinction between NULL and
"".

Not a big objection; just a minor niggle, probably not worth a re-roll.

>         if (!strcmp(av[1], "add"))
>                 return add(ac - 1, av + 1, prefix);
>         if (!strcmp(av[1], "prune"))
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]