Petr Baudis <pasky@xxxxxxx> writes:
> diff --git a/builtin-mv.c b/builtin-mv.c
> index 4f65b5a..2970acc 100644
> --- a/builtin-mv.c
> +++ b/builtin-mv.c
> @@ -9,6 +9,7 @@
> #include "cache-tree.h"
> #include "string-list.h"
> #include "parse-options.h"
> +#include "submodule.h"
>
> static const char * const builtin_mv_usage[] = {
> "git mv [options] <source>... <destination>",
> @@ -49,6 +50,24 @@ static const char *add_slash(const char *path)
> return path;
> }
>
> +static int ce_is_gitlink(int i)
> +{
> + return i < 0 ? 0 : S_ISGITLINK(active_cache[i]->ce_mode);
> +}
This interface itself is ugly (why should a caller pass "it is unmerged or
does not exist" without checking?), and it also makes the hunk that begins
at 84/105 ugly. Why not "path_is_gitlink(const char*)" and run
cache_name_pos() here instead?
The interface, even if it is internal, should be done with a better taste
than that, even though I understand that you wanted to reuse the cache_pos
for the source one while you check.
Oh, by the way, what should happen when you have an unmerged path in the
index and say "git mv path elsewhere" (this question is *not* limited to
submodules). Compared to that, what _does_ happen with the current code,
and with your patch?
--
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
