Re: [PATCH 24/25] worktree move: accept destination as directory

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

 



On Wed, May 11, 2016 at 11:43 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Wed, Apr 13, 2016 at 9:15 AM, Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>> Similar to "mv a b/", which is actually "mv a b/a", we extract basename
>> of source worktree and create a directory of the same name at
>> destination if dst path is a directory.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
>> ---
>> diff --git a/builtin/worktree.c b/builtin/worktree.c
>> @@ -538,7 +538,13 @@ static int move_worktree(int ac, const char **av, const char *prefix)
>> -       if (file_exists(dst.buf))
>> +       if (is_directory(dst.buf))
>> +               /*
>> +                * keep going, dst will be appended after we get the
>> +                * source's absolute path
>> +                */
>> +               ;
>> +       else if (file_exists(dst.buf))
>>                 die(_("target '%s' already exists"), av[1]);
>> @@ -558,6 +564,17 @@ static int move_worktree(int ac, const char **av, const char *prefix)
>> +       if (is_directory(dst.buf)) {
>> +               const char *sep = strrchr(wt->path, '/');
>
> Does this need to take Windows into account?

wt->path comes from $GIT_DIR/worktrees/xxx/gitdir, which normally uses
forward slashes, so we should be safe. We already rely on forward
slashes in get_linked_worktree()

> Perhaps git_find_last_dir_sep()?

But this is probably a good thing to do anyway, to be more robust in
future. But it could confuse the reader later on why it's necessary
when backward slashes can't exist in wt->path. I don't know. Maybe
just have a comment that backward slashes can't never appear here?

There is also a potential problem with find_worktree_by_path(). I was
counting on real_path() to normalize paths and could simply do
strcmp_icase (or its new name, fspathcmp). But real_path() does not
seem to convert unify slashes. I will need to have a closer look at
this. Hopefully prefix_filename() already makes sure everything uses
forward slashes. Or maybe we could improve fspathcmp to see '/' and
'\' the same thing on Windows.
-- 
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



[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]