git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: git@vger.kernel.org
Cc: "Jonathan Nieder" <jrnieder@gmail.com>,
	"Jens Lehmann" <Jens.Lehmann@web.de>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Thomas Gummerer" <t.gummerer@gmail.com>
Subject: [PATCH v5 1/2] diff: move no-index detection to builtin/diff.c
Date: Sat, 14 Dec 2013 14:07:20 +0100	[thread overview]
Message-ID: <1387026441-14539-1-git-send-email-t.gummerer@gmail.com> (raw)
In-Reply-To: <20131214004347.GZ2311@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 Jonathan for reviewing the last round, this version addresses
those comments, fixing the error message when git diff --no-index is
run outside of a git directory and avoids some nesting as suggested.

 builtin/diff.c  | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 diff-no-index.c | 44 +-------------------------------------------
 diff.h          |  2 +-
 3 files changed, 52 insertions(+), 47 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index adb93a9..f49a938 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,58 @@ 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);
+	if (!no_index) {
+		/*
+		 * 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 && 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 (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

  parent reply	other threads:[~2013-12-14 13:07 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 12:05 [PATCH] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-09 15:16 ` Jonathan Nieder
2013-12-09 17:27   ` Jens Lehmann
2013-12-09 19:06     ` Jonathan Nieder
2013-12-09 19:14   ` Thomas Gummerer
2013-12-09 19:20     ` Jonathan Nieder
2013-12-09 20:40       ` [PATCH v2] " Thomas Gummerer
2013-12-09 20:55         ` Torsten Bögershausen
2013-12-09 20:57         ` Eric Sunshine
2013-12-09 21:17           ` Thomas Gummerer
2013-12-09 20:30 ` [PATCH] " Junio C Hamano
2013-12-09 21:13   ` Thomas Gummerer
2013-12-10 17:52   ` [PATCH v3 1/2] diff: move no-index detection to builtin/diff.c Thomas Gummerer
2013-12-10 17:52     ` [PATCH v3 2/2] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-10 18:18       ` Jonathan Nieder
2013-12-10 18:55         ` Thomas Gummerer
2013-12-10 18:16     ` [PATCH v3 1/2] diff: move no-index detection to builtin/diff.c Jonathan Nieder
2013-12-10 18:46       ` Thomas Gummerer
2013-12-11  9:58       ` [PATCH v4 " Thomas Gummerer
2013-12-11  9:58         ` [PATCH v4 2/2] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-12 20:25           ` Junio C Hamano
2013-12-14  0:44           ` Jonathan Nieder
2013-12-14  0:43         ` [PATCH v4 1/2] diff: move no-index detection to builtin/diff.c Jonathan Nieder
2013-12-14 10:42           ` Thomas Gummerer
2013-12-16 17:48             ` Junio C Hamano
2013-12-16 19:23               ` [PATCH 1/2] diff: add test for --no-index executed outside repo Thomas Gummerer
2013-12-16 19:23                 ` [PATCH 2/2] diff: avoid some nesting Thomas Gummerer
2013-12-16 19:42                 ` [PATCH 1/2] diff: add test for --no-index executed outside repo Junio C Hamano
2013-12-14 13:07           ` Thomas Gummerer [this message]
2013-12-14 13:07             ` [PATCH v5 2/2] diff: don't read index when --no-index is given Thomas Gummerer

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1387026441-14539-1-git-send-email-t.gummerer@gmail.com \
    --to=t.gummerer@gmail.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).