git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 1/2] diff: move no-index detection to builtin/diff.c

From
Thomas Gummerer <t.gummerer@gmail.com>
Date
Dec 11, 2013, 09:58 UTC
Message-ID
<1386755923-22132-1-git-send-email-t.gummerer@gmail.com>
In-Reply-To
<20131210181655.GB2311@google.com>

Currently the --no-index option is parsed in diff_no_index(). Move the detection if a no-index diff should be executed to builtin/diff.c, where we can use it for executing diff_no_index() conditionally. This will also allow us to execute other operations conditionally, which will be done in the next patch.

There are no functional changes.
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---

Thanks to Jonathan for the suggestions in the previous round. Since the last roud I've made the no_index detection easier to read, and use two different values for no_index to indicate whether the no_index option is given explicitly, or if a no_index diff should be executed implicitly.

 builtin/diff.c  | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 diff-no-index.c | 44 +-------------------------------------------
 diff.h          |  2 +-
 3 files changed, 51 insertions(+), 47 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..da69e4a 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -16,6 +16,9 @@
 #include "submodule.h"
 #include "sha1-array.h"
 
+#define DIFF_NO_INDEX_EXPLICIT 1
+#define DIFF_NO_INDEX_IMPLICIT 2
+
 struct blobinfo {
 	unsigned char sha1[20];
 	const char *name;
@@ -257,7 +260,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	int blobs = 0, paths = 0;
 	const char *path = NULL;
 	struct blobinfo blob[2];
-	int nongit;
+	int nongit = 0, no_index = 0;
 	int result = 0;
 
 	/*
@@ -283,14 +286,57 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	 * Other cases are errors.
 	 */
 
+	/* Were we asked to do --no-index explicitly? */
+	for (i = 1; i < argc; i++) {
+		if (!strcmp(argv[i], "--")) {
+			i++;
+			break;
+		}
+		if (!strcmp(argv[i], "--no-index"))
+			no_index = DIFF_NO_INDEX_EXPLICIT;
+		if (argv[i][0] != '-')
+			break;
+	}
+
 	prefix = setup_git_directory_gently(&nongit);
+	/*
+	 * Treat git diff with at least one path outside of the
+	 * repo the same as if the command would have been executed
+	 * outside of a git repository.  In this case it behaves
+	 * the same way as "git diff --no-index <a> <b>", which acts
+	 * as a colourful "diff" replacement.
+	 */
+	if (nongit || ((argc == i + 2) &&
+		       (!path_inside_repo(prefix, argv[i]) ||
+			!path_inside_repo(prefix, argv[i + 1]))))
+		no_index = DIFF_NO_INDEX_IMPLICIT;
+
 	gitmodules_config();
 	git_config(git_diff_ui_config, NULL);
 
 	init_revisions(&rev, prefix);
 
-	/* If this is a no-index diff, just run it and exit there. */
-	diff_no_index(&rev, argc, argv, nongit, prefix);
+	if (no_index) {
+		if (argc != i + 2) {
+			if (no_index == DIFF_NO_INDEX_IMPLICIT) {
+				/*
+				 * There was no --no-index and there were not two
+				 * paths. It is possible that the user intended
+				 * to do an inside-repository operation.
+				 */
+				fprintf(stderr, "Not a git repository\n");
+				fprintf(stderr,
+					"To compare two paths outside a working tree:\n");
+			}
+			/* Give the usage message for non-repository usage and exit. */
+			usagef("git diff %s <path> <path>",
+			       no_index == DIFF_NO_INDEX_EXPLICIT ?
+					"--no-index" : "[--no-index]");
+
+		}
+		/* If this is a no-index diff, just run it and exit there. */
+		diff_no_index(&rev, argc, argv, prefix);
+	}
 
 	/* Otherwise, we are doing the usual "git" diff */
 	rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
diff --git a/diff-no-index.c b/diff-no-index.c
index 00a8eef..33e5982 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -183,54 +183,12 @@ static int queue_diff(struct diff_options *o,
 
 void diff_no_index(struct rev_info *revs,
 		   int argc, const char **argv,
-		   int nongit, const char *prefix)
+		   const char *prefix)
 {
 	int i, prefixlen;
-	int no_index = 0;
 	unsigned deprecated_show_diff_q_option_used = 0;
 	const char *paths[2];
 
-	/* Were we asked to do --no-index explicitly? */
-	for (i = 1; i < argc; i++) {
-		if (!strcmp(argv[i], "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(argv[i], "--no-index"))
-			no_index = 1;
-		if (argv[i][0] != '-')
-			break;
-	}
-
-	if (!no_index && !nongit) {
-		/*
-		 * Inside a git repository, without --no-index.  Only
-		 * when a path outside the repository is given,
-		 * e.g. "git diff /var/tmp/[12]", or "git diff
-		 * Makefile /var/tmp/Makefile", allow it to be used as
-		 * a colourful "diff" replacement.
-		 */
-		if ((argc != i + 2) ||
-		    (path_inside_repo(prefix, argv[i]) &&
-		     path_inside_repo(prefix, argv[i+1])))
-			return;
-	}
-	if (argc != i + 2) {
-		if (!no_index) {
-			/*
-			 * There was no --no-index and there were not two
-			 * paths. It is possible that the user intended
-			 * to do an inside-repository operation.
-			 */
-			fprintf(stderr, "Not a git repository\n");
-			fprintf(stderr,
-				"To compare two paths outside a working tree:\n");
-		}
-		/* Give the usage message for non-repository usage and exit. */
-		usagef("git diff %s <path> <path>",
-		       no_index ? "--no-index" : "[--no-index]");
-	}
-
 	diff_setup(&revs->diffopt);
 	for (i = 1; i < argc - 2; ) {
 		int j;
diff --git a/diff.h b/diff.h
index e342325..de105d3 100644
--- a/diff.h
+++ b/diff.h
@@ -330,7 +330,7 @@ extern int diff_flush_patch_id(struct diff_options *, unsigned char *);
 
 extern int diff_result_code(struct diff_options *, int);
 
-extern void diff_no_index(struct rev_info *, int, const char **, int, const char *);
+extern void diff_no_index(struct rev_info *, int, const char **, const char *);
 
 extern int index_differs_from(const char *def, int diff_flags);
 

-- 
1.8.5.4.g8639e57
Previous: Thomas GummererNext: Thomas Gummerer
Message 19 of 30 in “diff: don't read index when --no-index is given”
  1. diff: don't read index when --no-index is givenThomas Gummerer, Dec 9, 2013
  2. Jonathan NiederDec 9, 2013
  3. Jens LehmannDec 9, 2013
  4. Jonathan NiederDec 9, 2013
  5. Thomas GummererDec 9, 2013
  6. Jonathan NiederDec 9, 2013
  7. diff: don't read index when --no-index is givenThomas Gummerer, Dec 9, 2013
  8. Torsten BögershausenDec 9, 2013
  9. Eric SunshineDec 9, 2013
  10. Thomas GummererDec 9, 2013
  11. Junio C HamanoDec 9, 2013
  12. Thomas GummererDec 9, 2013
  13. 1/2 diff: move no-index detection to builtin/diff.cThomas Gummerer, Dec 10, 2013
  14. 2/2 diff: don't read index when --no-index is givenThomas Gummerer, Dec 10, 2013
  15. Jonathan NiederDec 10, 2013
  16. Thomas GummererDec 10, 2013
  17. Jonathan NiederDec 10, 2013
  18. Thomas GummererDec 10, 2013
  19. 1/2 diff: move no-index detection to builtin/diff.cThomas Gummerer, Dec 11, 2013
  20. 2/2 diff: don't read index when --no-index is givenThomas Gummerer, Dec 11, 2013
  21. Junio C HamanoDec 12, 2013
  22. Jonathan NiederDec 14, 2013
  23. Jonathan NiederDec 14, 2013
  24. Thomas GummererDec 14, 2013
  25. Junio C HamanoDec 16, 2013
  26. 1/2 diff: add test for --no-index executed outside repoThomas Gummerer, Dec 16, 2013
  27. 2/2 diff: avoid some nestingThomas Gummerer, Dec 16, 2013
  28. Junio C HamanoDec 16, 2013
  29. 1/2 diff: move no-index detection to builtin/diff.cThomas Gummerer, Dec 14, 2013
  30. 2/2 diff: don't read index when --no-index is givenThomas Gummerer, Dec 14, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.