Re: [PATCH 4/8] diff: support reading a file...

Re: [PATCH 4/8] diff: support reading a file from stdin via "-"

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:56

Johannes Schindelin [off-list ref] writes:

I see this purely for purposes of --no-index diff. And in that case, we 
only compare one file pair. Either one of them is from stdin, or it is 
not. Therefore, if memory gets tight, we cannot compare that file pair 
anyway, and have to error out.

Maybe I missed something obvious?

I think the responsibility to mark if a path whose name is "-"
is a filesystem object or stdin lies within the option parser.
If the user says "diff - a" or diff a -" then the user clearly
means the stdin, while if the user says "diff ./- a" it is a
path (and the later stages like populate_filespec() would see
canonicalized "-" as the filename).  The point is that the
disambiguation must be done by somebody, and I think the option
parser, who calls get_pathspec(), should be the one that does
it.

So we at least need a bit in diff_options that lets the option
parser to tell the later stages "if you see - in pathspec, the
user means stdin".  And my suggestion was that if you are going
to do that in the option parser anyway, you could read stdin in
the option parser and hang that data in diff_options, and the
presence of that stdin data (pointer being non NULL) can be used
as that signal.

I think the callers of populate_filespec() may need to pass
around diff_options as a parameter for the above to work, but
hopefully that should not be a rocket surgery.

Re: [PATCH 4/8] diff: support reading a file from stdin via "-"

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:42:56

Hi,

On Sun, 25 Feb 2007, Junio C Hamano wrote:

I think the callers of populate_filespec() may need to pass around 
diff_options as a parameter for the above to work, but hopefully that 
should not be a rocket surgery.

That's a good idea! Actually, that pointer can be the pointer in the 
diff_filespec itself!

This is my current version of [4/8] (Gosh, I really _love_ rebase -i ;-):

-- snipsnap --
[PATCH] diff: support reading a file from stdin via "-"

This allows you to say

	echo Hello World | git diff x -

to compare the contents of file "x" with the line "Hello World".
This automatically switches to --no-index mode.

Signed-off-by: Johannes Schindelin <redacted>
---

	This gives revs->max_count yet another meaning, oh well...

	The diff is a bit borked, because get_mode() and the first part 
	of the old queue_diff() are apparently a little similar...

 diff-lib.c |   92 ++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 65 insertions(+), 27 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 6678e22..9302d2b 100644
--- a/diff-lib.c
+++ b/diff-lib.c

@@ -30,22 +30,61 @@ static int read_directory(const char *path, struct path_list *list)
 	return 0;
 }
 
-static int queue_diff(struct diff_options *o,
-		const char *name1, const char *name2)
+static int get_mode(const char *path, int *mode, int seen_dashdash)
 {
 	struct stat st;
-	int mode1 = 0, mode2 = 0;
 
-	if (name1) {
-		if (stat(name1, &st))
-			return error("Could not access '%s'", name1);
-		mode1 = st.st_mode;
+	if (!path)
+		*mode = 0;
+	else if (!seen_dashdash && !strcmp(path, "-"))
+		*mode = 020600;
+	else if (stat(path, &st))
+		return error("Could not access '%s'", path);
+	else
+		*mode = st.st_mode;
+	return 0;
+}
+
+static void read_stdin(void **buffer, unsigned long *size)
+{
+	char *buf = NULL;
+#define INCREMENT 1024
+	int i = INCREMENT;
+
+	*size = 0;
+	buf = NULL;
+	while (i == INCREMENT) {
+		buf = xrealloc(buf, *size + INCREMENT);
+		i = xread(0, buf + *size, INCREMENT);
+		*size += i;
 	}
-	if (name2) {
-		if (stat(name2, &st))
-			return error("Could not access '%s'", name2);
-		mode2 = st.st_mode;
+	*buffer = buf;
+}
+
+static struct diff_filespec *make_diff_filespec(const char *path, int mode)
+{
+	struct diff_filespec *result;
+	if (!path)
+		path = "/dev/null";
+	result = alloc_filespec(path);
+	fill_filespec(result, null_sha1, mode);
+	if (mode == 020600) {
+		read_stdin(&result->data, &result->size);
+		result->mode = 0100644;
+		result->should_munmap = 0;
+		result->should_free = 1;
 	}
+	return result;
+}
+
+static int queue_diff(struct diff_options *o,
+		const char *name1, const char *name2, int seen_dashdash)
+{
+	int mode1 = 0, mode2 = 0;
+
+	if (get_mode(name1, &mode1, seen_dashdash) ||
+			get_mode(name2, &mode2, seen_dashdash))
+		return -1;
 
 	if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))
 		return error("file/directory conflict: %s, %s", name1, name2);

@@ -106,7 +145,7 @@ static int queue_diff(struct diff_options *o,
 						PATH_MAX - len2);
 			}
 
-			ret = queue_diff(o, n1, n2);
+			ret = queue_diff(o, n1, n2, 1);
 		}
 		path_list_clear(&p1, 0);
 		path_list_clear(&p2, 0);

@@ -122,14 +161,8 @@ static int queue_diff(struct diff_options *o,
 			tmp_c = name1; name1 = name2; name2 = tmp_c;
 		}
 
-		if (!name1)
-			name1 = "/dev/null";
-		if (!name2)
-			name2 = "/dev/null";
-		d1 = alloc_filespec(name1);
-		d2 = alloc_filespec(name2);
-		fill_filespec(d1, null_sha1, mode1);
-		fill_filespec(d2, null_sha1, mode2);
+		d1 = make_diff_filespec(name1, mode1);
+		d2 = make_diff_filespec(name2, mode2);
 
 		diff_queue(&diff_queued_diff, d1, d2);
 		return 0;

@@ -222,11 +255,12 @@ static int is_outside_repo(const char *path, int nongit, const char *prefix)
 int setup_diff_no_index(struct rev_info *revs,
 		int argc, const char ** argv, int nongit, const char *prefix)
 {
-	int i;
+	int i, seen_dashdash = 0;
 	for (i = 1; i < argc; i++)
-		if (argv[i][0] != '-')
+		if (argv[i][0] != '-' || argv[i][1] == '\0')
 			break;
 		else if (!strcmp(argv[i], "--")) {
+			seen_dashdash = 1;
 			i++;
 			break;
 		} else if (i < argc - 3 && !strcmp(argv[i], "--no-index")) {

@@ -238,7 +272,7 @@ int setup_diff_no_index(struct rev_info *revs,
 		return -1;
 
 	diff_setup(&revs->diffopt);
-	for (i = 1; i < argc - 2; )
+	for (i = 1; i < argc - 2 - seen_dashdash; )
 		if (!strcmp(argv[i], "--no-index"))
 			i++;
 		else {

@@ -250,22 +284,26 @@ int setup_diff_no_index(struct rev_info *revs,
 		}
 	revs->diffopt.paths = argv + argc - 2;
 	revs->diffopt.nr_paths = 2;
-	revs->max_count = -2;
+	revs->max_count = -2 - seen_dashdash;
 	return 0;
 }
 
 int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)
 {
-	int silent_on_removed;
+	int silent_on_removed, seen_dashdash = 0;
 
-	if (handle_diff_files_args(revs, argc, argv, &silent_on_removed))
+	if (revs->max_count == -3) {
+		seen_dashdash = 1;
+		revs->max_count = -2;
+	} else if (handle_diff_files_args(revs, argc, argv,
+				&silent_on_removed))
 		return -1;
 
 	if (revs->max_count == -2) {
 		if (revs->diffopt.nr_paths != 2)
 			return error("need two files/directories with --no-index");
 		if (queue_diff(&revs->diffopt, revs->diffopt.paths[0],
-				revs->diffopt.paths[1]))
+				revs->diffopt.paths[1], seen_dashdash))
 			return -1;
 		diffcore_std(&revs->diffopt);
 		diff_flush(&revs->diffopt);

-- 
1.5.0.1.784.g105d9-dirty

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help