All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/8] builtin/mv.c cleanup
@ 2014-08-10  2:29 Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 1/8] mv: mark strings for translations Nguyễn Thái Ngọc Duy
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

Interdiff on v1 below. I'm not going with MOVE_UP_BY_ONE because [1]
seems to agree with the name CLOSE_GAP, but that should be a separate
topic (touching more than just mv.c)

[1] http://thread.gmane.org/gmane.comp.version-control.git/243675/focus=244390

-- 8< --
diff --git a/builtin/mv.c b/builtin/mv.c
index 4eb420b..bf513e0 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -58,11 +58,6 @@ static const char *add_slash(const char *path)
 	return path;
 }
 
-static void move_up_one(void *p, int nmemb, int size)
-{
-	memmove(p, (char*)p + size, nmemb * size);
-}
-
 static struct lock_file lock_file;
 #define SUBMODULE_WITH_GITDIR ((const char *)1)
 
@@ -71,9 +66,9 @@ static void prepare_move_submodule(const char *src, int first,
 {
 	struct strbuf submodule_dotgit = STRBUF_INIT;
 	if (!S_ISGITLINK(active_cache[first]->ce_mode))
-		die (_("Directory %s is in index and no submodule?"), src);
+		die(_("Directory %s is in index and no submodule?"), src);
 	if (!is_staging_gitmodules_ok())
-		die (_("Please, stage your changes to .gitmodules or stash them to proceed"));
+		die(_("Please stage your changes to .gitmodules or stash them to proceed"));
 	strbuf_addf(&submodule_dotgit, "%s/.git", src);
 	*submodule_gitfile = read_gitfile(submodule_dotgit.buf);
 	if (*submodule_gitfile)
@@ -91,7 +86,7 @@ static int index_range_of_same_dir(const char *src, int length,
 
 	first = cache_name_pos(src_w_slash, len_w_slash);
 	if (first >= 0)
-		die (_("%.*s is in index"), len_w_slash, src_w_slash);
+		die(_("%.*s is in index"), len_w_slash, src_w_slash);
 
 	first = -1 - first;
 	for (last = first; last < active_nr; last++) {
@@ -235,14 +230,18 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		if (!bad)
 			continue;
 		if (!ignore_errors)
-			die (_("%s, source=%s, destination=%s"),
+			die(_("%s, source=%s, destination=%s"),
 			     bad, src, dst);
 		if (--argc > 0) {
 			int n = argc - i;
-			move_up_one(source + i, n, sizeof(*source));
-			move_up_one(destination + i, n, sizeof(*destination));
-			move_up_one(modes + i, n, sizeof(*modes));
-			move_up_one(submodule_gitfile + i, n, sizeof(*submodule_gitfile));
+			memmove(source + i, source + i + 1,
+				n * sizeof(char *));
+			memmove(destination + i, destination + i + 1,
+				n * sizeof(char *));
+			memmove(modes + i, modes + i + 1,
+				n * sizeof(enum update_mode));
+			memmove(submodule_gitfile + i, submodule_gitfile + i + 1,
+				n * sizeof(char *));
 			i--;
 		}
 	}
@@ -255,7 +254,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			printf(_("Renaming %s to %s\n"), src, dst);
 		if (!show_only && mode != INDEX) {
 			if (rename(src, dst) < 0 && !ignore_errors)
-				die_errno (_("renaming '%s' failed"), src);
+				die_errno(_("renaming '%s' failed"), src);
 			if (submodule_gitfile[i]) {
 				if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
 					connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
@@ -278,7 +277,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 
 	if (active_cache_changed &&
 	    write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
-			die(_("Unable to write new index file"));
+		die(_("Unable to write new index file"));
 
 	return 0;
 }
-- 8< --

Nguyễn Thái Ngọc Duy (8):
  mv: mark strings for translations
  mv: flatten error handling code block
  mv: split submodule move preparation code out
  mv: remove an "if" that's always true
  mv: move index search code out
  mv: unindent one level for directory move code
  mv: combine two if(s)
  mv: no SP between function name and the first opening parenthese

 builtin/mv.c | 171 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 85 insertions(+), 86 deletions(-)

-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 1/8] mv: mark strings for translations
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 2/8] mv: flatten error handling code block Nguyễn Thái Ngọc Duy
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 6ffe540..b892f63 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -108,7 +108,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		destination = internal_copy_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
 	} else {
 		if (argc != 1)
-			die("destination '%s' is not a directory", dest_path[0]);
+			die(_("destination '%s' is not a directory"), dest_path[0]);
 		destination = dest_path;
 	}
 
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/8] mv: flatten error handling code block
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 1/8] mv: mark strings for translations Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 3/8] mv: split submodule move preparation code out Nguyễn Thái Ngọc Duy
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index b892f63..c48129e 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -225,24 +225,22 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		else
 			string_list_insert(&src_for_dst, dst);
 
-		if (bad) {
-			if (ignore_errors) {
-				if (--argc > 0) {
-					memmove(source + i, source + i + 1,
-						(argc - i) * sizeof(char *));
-					memmove(destination + i,
-						destination + i + 1,
-						(argc - i) * sizeof(char *));
-					memmove(modes + i, modes + i + 1,
-						(argc - i) * sizeof(enum update_mode));
-					memmove(submodule_gitfile + i,
-						submodule_gitfile + i + 1,
-						(argc - i) * sizeof(char *));
-					i--;
-				}
-			} else
-				die (_("%s, source=%s, destination=%s"),
-				     bad, src, dst);
+		if (!bad)
+			continue;
+		if (!ignore_errors)
+			die (_("%s, source=%s, destination=%s"),
+			     bad, src, dst);
+		if (--argc > 0) {
+			int n = argc - i;
+			memmove(source + i, source + i + 1,
+				n * sizeof(char *));
+			memmove(destination + i, destination + i + 1,
+				n * sizeof(char *));
+			memmove(modes + i, modes + i + 1,
+				n * sizeof(enum update_mode));
+			memmove(submodule_gitfile + i, submodule_gitfile + i + 1,
+				n * sizeof(char *));
+			i--;
 		}
 	}
 
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/8] mv: split submodule move preparation code out
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 1/8] mv: mark strings for translations Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 2/8] mv: flatten error handling code block Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 4/8] mv: remove an "if" that's always true Nguyễn Thái Ngọc Duy
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

"Huh?" is removed from die() message.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index c48129e..524f4e5 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -61,6 +61,23 @@ static const char *add_slash(const char *path)
 static struct lock_file lock_file;
 #define SUBMODULE_WITH_GITDIR ((const char *)1)
 
+static void prepare_move_submodule(const char *src, int first,
+				   const char **submodule_gitfile)
+{
+	struct strbuf submodule_dotgit = STRBUF_INIT;
+	if (!S_ISGITLINK(active_cache[first]->ce_mode))
+		die(_("Directory %s is in index and no submodule?"), src);
+	if (!is_staging_gitmodules_ok())
+		die(_("Please stage your changes to .gitmodules or stash them to proceed"));
+	strbuf_addf(&submodule_dotgit, "%s/.git", src);
+	*submodule_gitfile = read_gitfile(submodule_dotgit.buf);
+	if (*submodule_gitfile)
+		*submodule_gitfile = xstrdup(*submodule_gitfile);
+	else
+		*submodule_gitfile = SUBMODULE_WITH_GITDIR;
+	strbuf_release(&submodule_dotgit);
+}
+
 int cmd_mv(int argc, const char **argv, const char *prefix)
 {
 	int i, gitmodules_modified = 0;
@@ -132,20 +149,10 @@ 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);
-			if (first >= 0) {
-				struct strbuf submodule_dotgit = STRBUF_INIT;
-				if (!S_ISGITLINK(active_cache[first]->ce_mode))
-					die (_("Huh? Directory %s is in index and no submodule?"), src);
-				if (!is_staging_gitmodules_ok())
-					die (_("Please, stage your changes to .gitmodules or stash them to proceed"));
-				strbuf_addf(&submodule_dotgit, "%s/.git", src);
-				submodule_gitfile[i] = read_gitfile(submodule_dotgit.buf);
-				if (submodule_gitfile[i])
-					submodule_gitfile[i] = xstrdup(submodule_gitfile[i]);
-				else
-					submodule_gitfile[i] = SUBMODULE_WITH_GITDIR;
-				strbuf_release(&submodule_dotgit);
-			} else {
+			if (first >= 0)
+				prepare_move_submodule(src, first,
+						       submodule_gitfile + i);
+			else {
 				const char *src_w_slash = add_slash(src);
 				int last, len_w_slash = length + 1;
 
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/8] mv: remove an "if" that's always true
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
                   ` (2 preceding siblings ...)
  2014-08-10  2:29 ` [PATCH v2 3/8] mv: split submodule move preparation code out Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 5/8] mv: move index search code out Nguyễn Thái Ngọc Duy
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

This is inside an "else" block of "if (last - first < 1)", so we know
that "last - first >= 1" when we come here. No need to check
"last - first > 0".

While at there, save "argc + last - first" to a variable to shorten
the statements a bit.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 524f4e5..0732355 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -175,22 +175,14 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 				if (last - first < 1)
 					bad = _("source directory is empty");
 				else {
-					int j, dst_len;
+					int j, dst_len, n;
 
-					if (last - first > 0) {
-						source = xrealloc(source,
-								(argc + last - first)
-								* sizeof(char *));
-						destination = xrealloc(destination,
-								(argc + last - first)
-								* sizeof(char *));
-						modes = xrealloc(modes,
-								(argc + last - first)
-								* sizeof(enum update_mode));
-						submodule_gitfile = xrealloc(submodule_gitfile,
-								(argc + last - first)
-								* sizeof(char *));
-					}
+					n = argc + last - first;
+					source = xrealloc(source, n * sizeof(char *));
+					destination = xrealloc(destination, n * sizeof(char *));
+					modes = xrealloc(modes, n * sizeof(enum update_mode));
+					submodule_gitfile =
+						xrealloc(submodule_gitfile, n * sizeof(char *));
 
 					dst = add_slash(dst);
 					dst_len = strlen(dst);
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 5/8] mv: move index search code out
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
                   ` (3 preceding siblings ...)
  2014-08-10  2:29 ` [PATCH v2 4/8] mv: remove an "if" that's always true Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 6/8] mv: unindent one level for directory move code Nguyễn Thái Ngọc Duy
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

"Huh?" is removed from die() message.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 0732355..dcfcb11 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -78,6 +78,29 @@ static void prepare_move_submodule(const char *src, int first,
 	strbuf_release(&submodule_dotgit);
 }
 
+static int index_range_of_same_dir(const char *src, int length,
+				   int *first_p, int *last_p)
+{
+	const char *src_w_slash = add_slash(src);
+	int first, last, len_w_slash = length + 1;
+
+	first = cache_name_pos(src_w_slash, len_w_slash);
+	if (first >= 0)
+		die(_("%.*s is in index"), len_w_slash, src_w_slash);
+
+	first = -1 - first;
+	for (last = first; last < active_nr; last++) {
+		const char *path = active_cache[last]->name;
+		if (strncmp(path, src_w_slash, len_w_slash))
+			break;
+	}
+	if (src_w_slash != src)
+		free((char *)src_w_slash);
+	*first_p = first;
+	*last_p = last;
+	return last - first;
+}
+
 int cmd_mv(int argc, const char **argv, const char *prefix)
 {
 	int i, gitmodules_modified = 0;
@@ -153,25 +176,10 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 				prepare_move_submodule(src, first,
 						       submodule_gitfile + i);
 			else {
-				const char *src_w_slash = add_slash(src);
-				int last, len_w_slash = length + 1;
+				int last;
 
 				modes[i] = WORKING_DIRECTORY;
-
-				first = cache_name_pos(src_w_slash, len_w_slash);
-				if (first >= 0)
-					die (_("Huh? %.*s is in index?"),
-							len_w_slash, src_w_slash);
-
-				first = -1 - first;
-				for (last = first; last < active_nr; last++) {
-					const char *path = active_cache[last]->name;
-					if (strncmp(path, src_w_slash, len_w_slash))
-						break;
-				}
-				if (src_w_slash != src)
-					free((char *)src_w_slash);
-
+				index_range_of_same_dir(src, length, &first, &last);
 				if (last - first < 1)
 					bad = _("source directory is empty");
 				else {
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 6/8] mv: unindent one level for directory move code
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
                   ` (4 preceding siblings ...)
  2014-08-10  2:29 ` [PATCH v2 5/8] mv: move index search code out Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  2014-08-11 18:47   ` Junio C Hamano
  2014-08-10  2:29 ` [PATCH v2 7/8] mv: combine two if(s) Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 8/8] mv: no SP between function name and the first opening parenthese Nguyễn Thái Ngọc Duy
  7 siblings, 1 reply; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

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);
+			int first = cache_name_pos(src, length), last;
 			if (first >= 0)
 				prepare_move_submodule(src, first,
 						       submodule_gitfile + i);
-			else {
-				int last;
-
+			else if (index_range_of_same_dir(src, length,
+							 &first, &last) < 1) {
 				modes[i] = WORKING_DIRECTORY;
-				index_range_of_same_dir(src, length, &first, &last);
 				if (last - first < 1)
 					bad = _("source directory is empty");
-				else {
-					int j, dst_len, n;
+			} else { /* last - first >= 1 */
+				int j, dst_len, n;
 
-					n = argc + last - first;
-					source = xrealloc(source, n * sizeof(char *));
-					destination = xrealloc(destination, n * sizeof(char *));
-					modes = xrealloc(modes, n * sizeof(enum update_mode));
-					submodule_gitfile =
-						xrealloc(submodule_gitfile, n * sizeof(char *));
+				modes[i] = WORKING_DIRECTORY;
+				n = argc + last - first;
+				source = xrealloc(source, n * sizeof(char *));
+				destination = xrealloc(destination, n * sizeof(char *));
+				modes = xrealloc(modes, n * sizeof(enum update_mode));
+				submodule_gitfile = xrealloc(submodule_gitfile, n * sizeof(char *));
 
-					dst = add_slash(dst);
-					dst_len = strlen(dst);
+				dst = add_slash(dst);
+				dst_len = strlen(dst);
 
-					for (j = 0; j < last - first; j++) {
-						const char *path =
-							active_cache[first + j]->name;
-						source[argc + j] = path;
-						destination[argc + j] =
-							prefix_path(dst, dst_len,
-								path + length + 1);
-						modes[argc + j] = INDEX;
-						submodule_gitfile[argc + j] = NULL;
-					}
-					argc += last - first;
+				for (j = 0; j < last - first; j++) {
+					const char *path = active_cache[first + j]->name;
+					source[argc + j] = path;
+					destination[argc + j] =
+						prefix_path(dst, dst_len, path + length + 1);
+					modes[argc + j] = INDEX;
+					submodule_gitfile[argc + j] = NULL;
 				}
+				argc += last - first;
 			}
 		} else if (cache_name_pos(src, length) < 0)
 			bad = _("not under version control");
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 7/8] mv: combine two if(s)
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
                   ` (5 preceding siblings ...)
  2014-08-10  2:29 ` [PATCH v2 6/8] mv: unindent one level for directory move code Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  2014-08-10  2:29 ` [PATCH v2 8/8] mv: no SP between function name and the first opening parenthese Nguyễn Thái Ngọc Duy
  7 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index 988945c..cbe220f 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -275,10 +275,9 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 	if (gitmodules_modified)
 		stage_updated_gitmodules();
 
-	if (active_cache_changed) {
-		if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
-			die(_("Unable to write new index file"));
-	}
+	if (active_cache_changed &&
+	    write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
+		die(_("Unable to write new index file"));
 
 	return 0;
 }
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 8/8] mv: no SP between function name and the first opening parenthese
  2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
                   ` (6 preceding siblings ...)
  2014-08-10  2:29 ` [PATCH v2 7/8] mv: combine two if(s) Nguyễn Thái Ngọc Duy
@ 2014-08-10  2:29 ` Nguyễn Thái Ngọc Duy
  7 siblings, 0 replies; 12+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2014-08-10  2:29 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/mv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/mv.c b/builtin/mv.c
index cbe220f..bf513e0 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -230,7 +230,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 		if (!bad)
 			continue;
 		if (!ignore_errors)
-			die (_("%s, source=%s, destination=%s"),
+			die(_("%s, source=%s, destination=%s"),
 			     bad, src, dst);
 		if (--argc > 0) {
 			int n = argc - i;
@@ -254,7 +254,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 			printf(_("Renaming %s to %s\n"), src, dst);
 		if (!show_only && mode != INDEX) {
 			if (rename(src, dst) < 0 && !ignore_errors)
-				die_errno (_("renaming '%s' failed"), src);
+				die_errno(_("renaming '%s' failed"), src);
 			if (submodule_gitfile[i]) {
 				if (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)
 					connect_work_tree_and_git_dir(dst, submodule_gitfile[i]);
-- 
2.1.0.rc0.78.gc0d8480

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 6/8] mv: unindent one level for directory move code
  2014-08-10  2:29 ` [PATCH v2 6/8] mv: unindent one level for directory move code Nguyễn Thái Ngọc Duy
@ 2014-08-11 18:47   ` Junio C Hamano
  2014-08-12 14:57     ` Duy Nguyen
  0 siblings, 1 reply; 12+ messages in thread
From: Junio C Hamano @ 2014-08-11 18:47 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git

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

> 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;

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 6/8] mv: unindent one level for directory move code
  2014-08-11 18:47   ` Junio C Hamano
@ 2014-08-12 14:57     ` Duy Nguyen
  2014-08-12 17:56       ` Junio C Hamano
  0 siblings, 1 reply; 12+ messages in thread
From: Duy Nguyen @ 2014-08-12 14:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List

On Tue, Aug 12, 2014 at 1:47 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> 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?

No you're right.

>> +                     } else { /* last - first >= 1 */
>> +                             int j, dst_len, n;
>
>> +                             modes[i] = WORKING_DIRECTORY;
>> +                             n = argc + last - first;
>> ...
>
> Otherwise, perhaps squash this in?

Sure. But I'm having second thought about this series.

mv.c is centered around files on worktree, which makes it pretty hard
if we want to make it more like rm.c where index and worktree entries
are treated more equally and pathspec is applied. Doing that in mv.c
would require a lot more reorganization that makes this series
obsolete. But on the other hand, I'm not even sure if I have time to
pursue that. So..
-- 
Duy

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 6/8] mv: unindent one level for directory move code
  2014-08-12 14:57     ` Duy Nguyen
@ 2014-08-12 17:56       ` Junio C Hamano
  0 siblings, 0 replies; 12+ messages in thread
From: Junio C Hamano @ 2014-08-12 17:56 UTC (permalink / raw)
  To: Duy Nguyen; +Cc: Git Mailing List

Duy Nguyen <pclouds@gmail.com> writes:

>> Otherwise, perhaps squash this in?
>
> Sure. But I'm having second thought about this series.
>
> mv.c is centered around files on worktree, which makes it pretty hard
> if we want to make it more like rm.c where index and worktree entries
> are treated more equally and pathspec is applied. Doing that in mv.c
> would require a lot more reorganization that makes this series
> obsolete.

Well, you may have started these as preparatory clean-up, but taken
alone these are purely making the result more readable without
changing any behaviour, so let's get them right first.

Thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-08-12 17:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-10  2:29 [PATCH v2 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
2014-08-10  2:29 ` [PATCH v2 1/8] mv: mark strings for translations Nguyễn Thái Ngọc Duy
2014-08-10  2:29 ` [PATCH v2 2/8] mv: flatten error handling code block Nguyễn Thái Ngọc Duy
2014-08-10  2:29 ` [PATCH v2 3/8] mv: split submodule move preparation code out Nguyễn Thái Ngọc Duy
2014-08-10  2:29 ` [PATCH v2 4/8] mv: remove an "if" that's always true Nguyễn Thái Ngọc Duy
2014-08-10  2:29 ` [PATCH v2 5/8] mv: move index search code out Nguyễn Thái Ngọc Duy
2014-08-10  2:29 ` [PATCH v2 6/8] mv: unindent one level for directory move code Nguyễn Thái Ngọc Duy
2014-08-11 18:47   ` Junio C Hamano
2014-08-12 14:57     ` Duy Nguyen
2014-08-12 17:56       ` Junio C Hamano
2014-08-10  2:29 ` [PATCH v2 7/8] mv: combine two if(s) Nguyễn Thái Ngọc Duy
2014-08-10  2:29 ` [PATCH v2 8/8] mv: no SP between function name and the first opening parenthese Nguyễn Thái Ngọc Duy

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.