threads / patch / 21685

patch, 3 parts"log --stdin" updates

Subject: [PATCH 0/3] "log --stdin" updates

## tl;dr

6 messages between Nov 20, 2009 and Nov 20, 2009. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Junio C Hamano· Nov 20, 2009, 11:25 UTC · lore

These three come on top of 61ab67a (Teach --stdin option to "log" family, 2009-11-03), which gave "--stdin" to the log family of commands (e.g. log, rev-list). The earlier patch allowed you to feed only the revs (which was what gitk originally wanted), but after zero or more revs (one rev per line), you can now feed a line that consists of "--" and then pathspecs (one path per line).

The whole series probably need to be updated to learn -z option to read NUL terminated input.

Junio C Hamano (3):
  read_revision_from_stdin(): use strbuf
  setup_revisions(): do not call get_pathspec() too early
  Make --stdin option to "log" family read also pathspecs
 revision.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 files changed, 82 insertions(+), 13 deletions(-)
Junio C Hamano· Nov 20, 2009, 11:25 UTC · re: Junio C Hamano · lore

[PATCH 1/3] read_revision_from_stdin(): use strbuf

It is so 2005 (and Linus ;-) to have a fixed 1000-byte buffer that reads from the user. Let's use strbuf to unlimit the input length.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 revision.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)
Show changes to revision.c +10 −8
diff --git a/revision.c b/revision.c
index 45c5de8..f5b735f 100644
--- a/revision.c
+++ b/revision.c
@@ -955,19 +955,21 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 
 static void read_revisions_from_stdin(struct rev_info *revs)
 {
-	char line[1000];
+	struct strbuf sb;
 
-	while (fgets(line, sizeof(line), stdin) != NULL) {
-		int len = strlen(line);
-		if (len && line[len - 1] == '\n')
-			line[--len] = '\0';
+	strbuf_init(&sb, 1000);
+	while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
+		int len = sb.len;
+		if (len && sb.buf[len - 1] == '\n')
+			sb.buf[--len] = '\0';
 		if (!len)
 			break;
-		if (line[0] == '-')
+		if (sb.buf[0] == '-')
 			die("options not supported in --stdin mode");
-		if (handle_revision_arg(line, revs, 0, 1))
-			die("bad revision '%s'", line);
+		if (handle_revision_arg(sb.buf, revs, 0, 1))
+			die("bad revision '%s'", sb.buf);
 	}
+	strbuf_release(&sb);
 }
 
 static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
-- 
1.6.5.3.342.g14bb9
Junio C Hamano· Nov 20, 2009, 11:25 UTC · re: Junio C Hamano · lore

[PATCH 2/3] setup_revisions(): do not call get_pathspec() too early

This is necessary because in the next patch I'll allow pathspecs to be fed from the standard input, and pathspecs taken from the command line (and converted via get_pathspec() already) in revs->prune_data too early gets in the way when we want to append from the standard input.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 revision.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)
Show changes to revision.c +6 −3
diff --git a/revision.c b/revision.c
index f5b735f..4410a45 100644
--- a/revision.c
+++ b/revision.c
@@ -1230,6 +1230,7 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
 {
 	int i, flags, left, seen_dashdash, read_from_stdin;
+	const char **prune_data = NULL;
 
 	/* First, search for "--" */
 	seen_dashdash = 0;
@@ -1240,7 +1241,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		argv[i] = NULL;
 		argc = i;
 		if (argv[i + 1])
-			revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+			prune_data = argv + i + 1;
 		seen_dashdash = 1;
 		break;
 	}
@@ -1321,12 +1322,14 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 			for (j = i; j < argc; j++)
 				verify_filename(revs->prefix, argv[j]);
 
-			revs->prune_data = get_pathspec(revs->prefix,
-							argv + i);
+			prune_data = argv + i;
 			break;
 		}
 	}
 
+	if (prune_data)
+		revs->prune_data = get_pathspec(revs->prefix, prune_data);
+
 	if (revs->def == NULL)
 		revs->def = def;
 	if (revs->show_merge)
-- 
1.6.5.3.342.g14bb9
Junio C Hamano· Nov 20, 2009, 11:25 UTC · re: Junio C Hamano · lore

[PATCH 3/3] Make --stdin option to "log" family read also pathspecs

Similar to the command line arguments, after giving zero or more revs, you can feed a line "--" and then feed pathspecs one at a time.

With this
	(
		echo ^maint
		echo --
		echo Documentation
	) | git log --stat --oneline --stdin master -- t
lists commits that touch Documentation/ or t/ between maint and master.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 revision.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 68 insertions(+), 4 deletions(-)
Show changes to revision.c +68 −4
diff --git a/revision.c b/revision.c
index 4410a45..8750c20 100644
--- a/revision.c
+++ b/revision.c
@@ -953,9 +953,38 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	return 0;
 }
 
-static void read_revisions_from_stdin(struct rev_info *revs)
+static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb, const char ***prune_data)
+{
+	const char **prune = *prune_data;
+	int prune_nr;
+	int prune_alloc;
+
+	/* count existing ones */
+	if (!prune)
+		prune_nr = 0;
+	else
+		for (prune_nr = 0; prune[prune_nr]; prune_nr++)
+			;
+	prune_alloc = prune_nr; /* not really, but we do not know */
+
+	while (strbuf_getwholeline(sb, stdin, '\n') != EOF) {
+		int len = sb->len;
+		if (len && sb->buf[len - 1] == '\n')
+			sb->buf[--len] = '\0';
+		ALLOC_GROW(prune, prune_nr+1, prune_alloc);
+		prune[prune_nr++] = xstrdup(sb->buf);
+	}
+	if (prune) {
+		ALLOC_GROW(prune, prune_nr+1, prune_alloc);
+		prune[prune_nr] = NULL;
+	}
+	*prune_data = prune;
+}
+
+static void read_revisions_from_stdin(struct rev_info *revs, const char ***prune)
 {
 	struct strbuf sb;
+	int seen_dashdash = 0;
 
 	strbuf_init(&sb, 1000);
 	while (strbuf_getwholeline(&sb, stdin, '\n') != EOF) {
@@ -964,11 +993,18 @@ static void read_revisions_from_stdin(struct rev_info *revs)
 			sb.buf[--len] = '\0';
 		if (!len)
 			break;
-		if (sb.buf[0] == '-')
+		if (sb.buf[0] == '-') {
+			if (len == 2 && sb.buf[1] == '-') {
+				seen_dashdash = 1;
+				break;
+			}
 			die("options not supported in --stdin mode");
+		}
 		if (handle_revision_arg(sb.buf, revs, 0, 1))
 			die("bad revision '%s'", sb.buf);
 	}
+	if (seen_dashdash)
+		read_pathspec_from_stdin(revs, &sb, prune);
 	strbuf_release(&sb);
 }
 
@@ -1220,6 +1256,34 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 	ctx->argc -= n;
 }
 
+static void append_prune_data(const char ***prune_data, const char **av)
+{
+	const char **prune = *prune_data;
+	int prune_nr;
+	int prune_alloc;
+
+	if (!prune) {
+		*prune_data = av;
+		return;
+	}
+
+	/* count existing ones */
+	for (prune_nr = 0; prune[prune_nr]; prune_nr++)
+		;
+	prune_alloc = prune_nr; /* not really, but we do not know */
+
+	while (*av) {
+		ALLOC_GROW(prune, prune_nr+1, prune_alloc);
+		prune[prune_nr++] = *av;
+		av++;
+	}
+	if (prune) {
+		ALLOC_GROW(prune, prune_nr+1, prune_alloc);
+		prune[prune_nr] = NULL;
+	}
+	*prune_data = prune;
+}
+
 /*
  * Parse revision information, filling in the "rev_info" structure,
  * and removing the used arguments from the argument list.
@@ -1294,7 +1358,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				}
 				if (read_from_stdin++)
 					die("--stdin given twice?");
-				read_revisions_from_stdin(revs);
+				read_revisions_from_stdin(revs, &prune_data);
 				continue;
 			}
 
@@ -1322,7 +1386,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 			for (j = i; j < argc; j++)
 				verify_filename(revs->prefix, argv[j]);
 
-			prune_data = argv + i;
+			append_prune_data(&prune_data, argv + i);
 			break;
 		}
 	}
-- 
1.6.5.3.342.g14bb9
Jeff King· Nov 20, 2009, 14:40 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/3] "log --stdin" updates

On Fri, Nov 20, 2009 at 03:25:12AM -0800, Junio C Hamano wrote:
Show 6 quoted lines
> These three come on top of 61ab67a (Teach --stdin option to "log" family,
> 2009-11-03), which gave "--stdin" to the log family of commands (e.g. log,
> rev-list).  The earlier patch allowed you to feed only the revs (which was
> what gitk originally wanted), but after zero or more revs (one rev per
> line), you can now feed a line that consists of "--" and then pathspecs
> (one path per line).

This looks like a good API feature, though I am curious about the missing "4/3" that was the motivation (of course, with a feature like this, it may be for your out-of-git script, but as I said, I am curious :) ). Is it a response to

  http://article.gmane.org/gmane.comp.version-control.git/133268
?

The implementation looks good from my cursory reading. Tests and docs are of course missing. We don't seem to have any rev-list --stdin tests already. Maybe even something as simple as:

  git rev-list HEAD -- file >expect &&
  (echo HEAD; echo --; echo file) | git rev-list --stdin >actual &&
  test_cmp expect actual

For docs, maybe squash this in (it mentions the pathspecs, but also loosens --stdin to be available for "git log" and friends, which should have been part of 61ab67a).

Show changes to Documentation/rev-list-options.txt +4 −2
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index bf66116..b44fdd9 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -243,12 +243,14 @@ endif::git-rev-list[]
 	Pretend as if all the refs in `$GIT_DIR/refs/remotes` are listed
 	on the command line as '<commit>'.
 
-ifdef::git-rev-list[]
 --stdin::
 
 	In addition to the '<commit>' listed on the command
-	line, read them from the standard input.
+	line, read them from the standard input. If a '--' separator is
+	seen, stop reading commits and start reading paths to limit the
+	result.
 
+ifdef::git-rev-list[]
 --quiet::
 
 	Don't print anything to standard output.  This form
Junio C Hamano· Nov 20, 2009, 17:59 UTC · re: Jeff King · lore

Re: [PATCH 0/3] "log --stdin" updates

Jeff King <peff@peff.net> writes:
Show 17 quoted lines
> On Fri, Nov 20, 2009 at 03:25:12AM -0800, Junio C Hamano wrote:
>
>> These three come on top of 61ab67a (Teach --stdin option to "log" family,
>> 2009-11-03), which gave "--stdin" to the log family of commands (e.g. log,
>> rev-list).  The earlier patch allowed you to feed only the revs (which was
>> what gitk originally wanted), but after zero or more revs (one rev per
>> line), you can now feed a line that consists of "--" and then pathspecs
>> (one path per line).
>
> This looks like a good API feature, though I am curious about the
> missing "4/3" that was the motivation (of course, with a feature
> like this, it may be for your out-of-git script, but as I said, I am
> curious :) ). Is it a response to
>
>   http://article.gmane.org/gmane.comp.version-control.git/133268
>
> ?

No. This series is a follow-up to $gmane/131971; I was suspicious that I missed something for the API feature to be complete back then (and the patch was not signed-off partly for that reason), and I found one thing that was lacking.

← back to recent threads