From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Tue, 10 May 2011 13:30:37 GMT
Subject: Re: [PATCH 7/9] mv ":" ":" is like moving nothing from nowhere to nowhere
Message-ID: <BANLkTinBv+aRfgdnFKM-=yedug+WtseXsQ@mail.gmail.com>
In-Reply-To: <1305006678-4051-8-git-send-email-gitster@pobox.com>

On Tue, May 10, 2011 at 12:51 PM, Junio C Hamano <gitster@pobox.com> wrote:
> diff --git a/builtin/mv.c b/builtin/mv.c
> index 93e8995..38af9f0 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -77,8 +77,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>                die("index file corrupt");
>
>        source = copy_pathspec(prefix, argv, argc, 0);
> -       modes = xcalloc(argc, sizeof(enum update_mode));
> +       if (!source)
> +               die("copying from nowhere?");
> +       modes = xcalloc(count_pathspec(source), sizeof(enum update_mode));
>        dest_path = copy_pathspec(prefix, argv + argc, 1, 0);
> +       if (!dest_path)
> +               die("copying to nowhere?");
>

This command is interesting. Note to self: adding support for "git mv
'*.c' to/there".

I think we could take get_pathspec() out of copy_pathspec() and call
it just once (in case get_pathspec() or its successor does fancy
things), something roughly like this

diff --git a/builtin/mv.c b/builtin/mv.c
index 40f33ca..a032789 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -32,7 +32,7 @@ static const char **copy_pathspec(const char
*prefix, const char **pathspec,
 			result[i] = base_name ? strdup(basename(it)) : it;
 		}
 	}
-	return get_pathspec(prefix, result);
+	return result;
 }

 static const char *add_slash(const char *path)
@@ -60,7 +60,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		OPT_BOOLEAN('k', NULL, &ignore_errors, "skip move/rename errors"),
 		OPT_END(),
 	};
-	const char **source, **destination, **dest_path;
+	const char **pathspec, **source, **destination, **dest_path;
 	enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
 	struct stat st;
 	struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
@@ -69,16 +69,20 @@ int cmd_mv(int argc, const char **argv, const char *prefix)

 	argc = parse_options(argc, argv, prefix, builtin_mv_options,
 			     builtin_mv_usage, 0);
-	if (--argc < 1)
+
+	pathspec = get_pathspec(prefix, argv);
+	argc = count_pathspec(pathspec);
+
+	if (--argc < 1 || !pathspec)
 		usage_with_options(builtin_mv_usage, builtin_mv_options);

 	newfd = hold_locked_index(&lock_file, 1);
 	if (read_cache() < 0)
 		die(_("index file corrupt"));

-	source = copy_pathspec(prefix, argv, argc, 0);
+	source = copy_pathspec(prefix, pathspec, argc, 0);
 	modes = xcalloc(argc, sizeof(enum update_mode));
-	dest_path = copy_pathspec(prefix, argv + argc, 1, 0);
+	dest_path = copy_pathspec(prefix, pathspec + argc, 1, 0);

 	if (dest_path[0][0] == '\0')
 		/* special case: "." was normalized to "" */
-- 
Duy