Re: [PATCH 1/2] builtin/mv: remove get_pathspec()
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Aug 6, 2015, 18:46 UTC
- Message-ID
- <CAPig+cRREzwRJV-az-p-r1E7JyDqDZ4GPvM1cQQzXXNyLjGPUQ@mail.gmail.com>
- In-Reply-To
- <1438885632-26470-2-git-send-email-sbeller@google.com>
On Thu, Aug 6, 2015 at 2:27 PM, Stefan Beller <sbeller@google.com> wrote:
> builtin/mv: remove get_pathspec()
Misleading. Perhaps rephrase as:
mv: drop dependency upon deprecated get_pathspec
> `get_pathspec` is deprecated and builtin/mv.c is its last caller, so > reimplement `get_pathspec` literally in builtin/mv.c
Curious. Since this is just moving code around, rather than doing the actual work to complete the final step as stated by the NEEDSWORK comment, isn't it just moving the "problem" from one location to another? Is it worth the code churn?
Show 20 quoted lines
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff --git a/builtin/mv.c b/builtin/mv.c
> index d1d4316..99e9b3c 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -10,6 +10,7 @@
> #include "string-list.h"
> #include "parse-options.h"
> #include "submodule.h"
> +#include "pathspec.h"
>
> static const char * const builtin_mv_usage[] = {
> N_("git mv [<options>] <source>... <destination>"),
> @@ -20,13 +21,16 @@ static const char * const builtin_mv_usage[] = {
> #define KEEP_TRAILING_SLASH 2
>
> static const char **internal_copy_pathspec(const char *prefix,
> - const char **pathspec,
> + const char **argv,What is this change about? It doesn't seem to be related to anything else in the patch or to its stated purpose, and makes the argument's purpose less clear, so it's not obvious why it is a good change.
Show 30 quoted lines
> int count, unsigned flags)
> {
> int i;
> + struct pathspec ps;
> const char **result = xmalloc((count + 1) * sizeof(const char *));
> - memcpy(result, pathspec, count * sizeof(const char *));
> + memcpy(result, argv, count * sizeof(const char *));
> result[count] = NULL;
> +
> + /* NEEDSWORK: Move these preprocessing steps into parse_pathspec */
> for (i = 0; i < count; i++) {
> int length = strlen(result[i]);
> int to_copy = length;
> @@ -42,7 +46,13 @@ static const char **internal_copy_pathspec(const char *prefix,
> result[i] = it;
> }
> }
> - return get_pathspec(prefix, result);
> +
> + parse_pathspec(&ps,
> + PATHSPEC_ALL_MAGIC &
> + ~(PATHSPEC_FROMTOP | PATHSPEC_LITERAL),
> + PATHSPEC_PREFER_CWD,
> + prefix, result);
> + return ps._raw;
> }
>
> static const char *add_slash(const char *path)
> --
> 2.5.0.239.g9728e1d.dirty