From: Junio C Hamano <gitster@pobox•com>
To: Petr Baudis <pasky@suse•cz>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] git mv: Support moving submodules
Date: Fri, 12 Sep 2008 15:19:07 -0700 [thread overview]
Message-ID: <7vprn9m2hw.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <20080912214129.14829.53058.stgit@localhost> (Petr Baudis's message of "Fri, 12 Sep 2008 23:42:31 +0200")
Petr Baudis <pasky@suse•cz> 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?
next prev parent reply other threads:[~2008-09-12 22:20 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-12 21:08 [PATCH 0/6] Submodule support for git mv and git rm Petr Baudis
2008-09-12 21:08 ` [PATCH 1/6] submodule.*: Introduce simple C interface for submodule lookup by path Petr Baudis
2008-09-12 21:23 ` Junio C Hamano
2008-09-12 21:35 ` Jakub Narebski
2008-09-12 21:58 ` Petr Baudis
2008-09-12 21:09 ` [PATCH 2/6] git rm: Support for removing submodules Petr Baudis
2008-09-12 21:49 ` Junio C Hamano
2008-09-12 21:59 ` Junio C Hamano
2008-09-12 22:24 ` Petr Baudis
2008-09-12 22:42 ` Junio C Hamano
2008-09-12 21:09 ` [PATCH 3/6] git mv: Support moving submodules Petr Baudis
2008-09-12 21:42 ` [PATCH] " Petr Baudis
2008-09-12 22:19 ` Junio C Hamano [this message]
2008-09-12 21:09 ` [PATCH 4/6] t7403: Submodule git mv, git rm testsuite Petr Baudis
2008-09-12 21:09 ` [PATCH 5/6] t7400: Add short "git submodule add" testsuite Petr Baudis
2008-09-12 21:09 ` [PATCH 6/6] git submodule add: Fix naming clash handling Petr Baudis
2008-09-13 2:24 ` Junio C Hamano
2008-09-13 11:32 ` Lars Hjemli
-- strict thread matches above, loose matches on Subject: below --
2008-07-17 22:31 [PATCH] git-mv: Keep moved index entries inact Petr Baudis
2008-07-17 22:34 ` [PATCH] git mv: Support moving submodules Petr Baudis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vprn9m2hw.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=pasky@suse$(echo .)cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox