Re: [PATCH v2 6/8] mv: unindent one level for directory move code

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 11, 2014, 18:47 UTC
Message-ID: <xmqq61hy7tii.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1407637776-19794-7-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 19 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  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?

> +			} else { /* last - first >= 1 */
> +				int j, dst_len, n;

> +				modes[i] = WORKING_DIRECTORY;
> +				n = argc + last - first;
> ...

Otherwise, perhaps squash this in?

diff --git a/builtin/mv.c b/builtin/mv.c
index bf513e0..bf784cb 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -172,15 +172,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			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) {
-				modes[i] = WORKING_DIRECTORY;
-				if (last - first < 1)
-					bad = _("source directory is empty");
-			} else { /* last - first >= 1 */
+							 &first, &last) < 1)
+				bad = _("source directory is empty");
+			else { /* last - first >= 1 */
 				int j, dst_len, n;
 
 				modes[i] = WORKING_DIRECTORY;