On Tue, Aug 12, 2014 at 1:47 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>
>> ---
>> builtin/mv.c | 47 +++++++++++++++++++++--------------------------
>> 1 file changed, 21 insertions(+), 26 deletions(-)
>>
>> diff --git a/builtin/mv.c b/builtin/mv.c
>> index dcfcb11..988945c 100644
>> --- a/builtin/mv.c
>> +++ b/builtin/mv.c
>> @@ -171,42 +171,37 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>> && lstat(dst, &st) == 0)
>> bad = _("cannot move directory over file");
>> else if (src_is_dir) {
>> + int first = cache_name_pos(src, length), last;
>> if (first >= 0)
>> prepare_move_submodule(src, first,
>> submodule_gitfile + i);
>> + else if (index_range_of_same_dir(src, length,
>> + &first, &last) < 1) {
>
> The function returns (last - first), so (last - first) < 1 holds
> inside this block, right?
>
>> modes[i] = WORKING_DIRECTORY;
>> if (last - first < 1)
>> bad = _("source directory is empty");
>
> Then do you need this conditional, or it is always bad here?
>
> If it is always bad, then modes[i] do not need to be assigned to,
> either, I think.
>
> Am I missing something?
No you're right.
>> + } else { /* last - first >= 1 */
>> + int j, dst_len, n;
>
>> + modes[i] = WORKING_DIRECTORY;
>> + n = argc + last - first;
>> ...
>
> Otherwise, perhaps squash this in?
Sure. But I'm having second thought about this series.
mv.c is centered around files on worktree, which makes it pretty hard
if we want to make it more like rm.c where index and worktree entries
are treated more equally and pathspec is applied. Doing that in mv.c
would require a lot more reorganization that makes this series
obsolete. But on the other hand, I'm not even sure if I have time to
pursue that. So..
--
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