[PATCH 0/2] Support `git reset --stdin`

This feature was missing, and made it cumbersome for third-party
tools to reset a lot of paths in one go.

Support for --stdin has been added, following builtin/checkout-index.c's
example.


Johannes Schindelin (2):
  reset: fix usage
  reset: support the --stdin option

 Documentation/git-reset.txt | 10 +++++++-
 builtin/reset.c             | 56 +++++++++++++++++++++++++++++++++++++++++++--
 t/t7107-reset-stdin.sh      | 33 ++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100755 t/t7107-reset-stdin.sh


base-commit: 8a36cd87b7c85a651ab388d403629865ffa3ba0d
Published-As: https://github.com/dscho/git/releases/tag/reset-stdin-v1
Fetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v1

-- 
2.10.1.513.g00ef6dd

The <tree-ish> parameter is actually optional (see man page).

Signed-off-by: Johannes Schindelin <redacted>
---
 builtin/reset.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index 5aa8607..c04ac07 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c

@@ -24,7 +24,7 @@
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
-	N_("git reset [-q] <tree-ish> [--] <paths>..."),
+	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
 	NULL
 };

-- 
2.10.1.513.g00ef6dd

Just like with other Git commands, this option makes it read the paths
from the standard input. It comes in handy when resetting many, many
paths at once and wildcards are not an option (e.g. when the paths are
generated by a tool).

Note: to keep things simple, we first parse the entire list and perform
the actual reset action only in a second phase.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-reset.txt | 10 +++++++-
 builtin/reset.c             | 56 +++++++++++++++++++++++++++++++++++++++++++--
 t/t7107-reset-stdin.sh      | 33 ++++++++++++++++++++++++++
 3 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100755 t/t7107-reset-stdin.sh

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 25432d9..533ef69 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt

@@ -8,7 +8,7 @@ git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [-q] [<tree-ish>] [--] <paths>...
+'git reset' [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>...
 'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 

@@ -97,6 +97,14 @@ OPTIONS
 --quiet::
 	Be quiet, only report errors.
 
+--stdin::
+	Instead of taking list of paths from the command line,
+	read list of paths from the standard input.  Paths are
+	separated by LF (i.e. one path per line) by default.
+
+-z::
+	Only meaningful with `--stdin`; paths are separated with
+	NUL character instead of LF.
 
 EXAMPLES
 --------

diff --git a/builtin/reset.c b/builtin/reset.c
index c04ac07..018735f 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c

@@ -21,10 +21,12 @@
 #include "parse-options.h"
 #include "unpack-trees.h"
 #include "cache-tree.h"
+#include "strbuf.h"
+#include "quote.h"
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
-	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+	N_("git reset [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>..."),
 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
 	NULL
 };

@@ -267,7 +269,9 @@ static int reset_refs(const char *rev, const struct object_id *oid)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0, unborn;
+	int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
+	char **stdin_paths = NULL;
+	int stdin_nr = 0, stdin_alloc = 0;
 	const char *rev;
 	struct object_id oid;
 	struct pathspec pathspec;

@@ -286,6 +290,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
+		OPT_BOOL('z', NULL, &nul_term_line,
+			N_("paths are separated with NUL character")),
+		OPT_BOOL(0, "stdin", &read_from_stdin,
+				N_("read paths from <stdin>")),
 		OPT_END()
 	};
 

@@ -295,6 +303,44 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	if (read_from_stdin) {
+		strbuf_getline_fn getline_fn = nul_term_line ?
+			strbuf_getline_nul : strbuf_getline_lf;
+		int flags = PATHSPEC_PREFER_FULL |
+			PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
+		struct strbuf buf = STRBUF_INIT;
+		struct strbuf unquoted = STRBUF_INIT;
+
+		if (patch_mode)
+			die(_("--stdin is incompatible with --patch"));
+
+		if (pathspec.nr)
+			die(_("--stdin is incompatible with path arguments"));
+
+		if (patch_mode)
+			flags |= PATHSPEC_PREFIX_ORIGIN;
+
+		while (getline_fn(&buf, stdin) != EOF) {
+			if (!nul_term_line && buf.buf[0] == '"') {
+				strbuf_reset(&unquoted);
+				if (unquote_c_style(&unquoted, buf.buf, NULL))
+					die(_("line is badly quoted"));
+				strbuf_swap(&buf, &unquoted);
+			}
+			ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+			stdin_paths[stdin_nr++] = xstrdup(buf.buf);
+			strbuf_reset(&buf);
+		}
+		strbuf_release(&unquoted);
+		strbuf_release(&buf);
+
+		ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+		stdin_paths[stdin_nr++] = NULL;
+		parse_pathspec(&pathspec, 0, flags, prefix,
+			       (const char **)stdin_paths);
+	} else if (nul_term_line)
+		die(_("-z requires --stdin"));
+
 	unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", oid.hash);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */

@@ -385,5 +431,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (!pathspec.nr)
 		remove_branch_state();
 
+	if (stdin_paths) {
+		while (stdin_nr)
+			free(stdin_paths[--stdin_nr]);
+		free(stdin_paths);
+	}
+
 	return update_ref_status;
 }

diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh
new file mode 100755
index 0000000..997dc42
--- /dev/null
+++ b/t/t7107-reset-stdin.sh

@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='reset --stdin'
+
+. ./test-lib.sh
+
+test_expect_success 'reset --stdin' '
+	test_commit hello &&
+	git rm hello.t &&
+	test -z "$(git ls-files hello.t)" &&
+	echo hello.t | git reset --stdin &&
+	test hello.t = "$(git ls-files hello.t)"
+'
+
+test_expect_success 'reset --stdin -z' '
+	test_commit world &&
+	git rm hello.t world.t &&
+	test -z "$(git ls-files hello.t world.t)" &&
+	printf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&
+	printf "hello.t\nworld.t\n" >expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--stdin requires --mixed' '
+	echo hello.t >list &&
+	test_must_fail git reset --soft --stdin <list &&
+	test_must_fail git reset --hard --stdin <list &&
+	git reset --mixed --stdin <list
+'
+
+test_done
+

-- 
2.10.1.513.g00ef6dd

On Tue, Oct 11, 2016 at 06:08:56PM +0200, Johannes Schindelin wrote:

This feature was missing, and made it cumbersome for third-party
tools to reset a lot of paths in one go.

Support for --stdin has been added, following builtin/checkout-index.c's
example.

Is git-reset the right layer to add scripting features? I thought we
usually pushed people doing mass index manipulation to use update-index
or read-tree. Is there something that reset makes easy that is hard with
those tools (I could imagine "--hard", but I see it is not supported
with your patch).

Not that I'm necessarily opposed to the patch, I was just surprised.

-Peff

W dniu 11.10.2016 o 18:09, Johannes Schindelin pisze:

I think you meant here

  +'git reset' [-q] [--stdin [-z]] [<tree-ish>]

Because you say "*Instead*" below.

And die if <paths> were supplied:

+		if (pathspec.nr)
+			die(_("--stdin is incompatible with path arguments"));

Of course you need to fix it in built-in synopsis as well:

+	N_("git reset [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>..."),
 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),

-- 
Jakub Narębski

Hi Kuba,

On Tue, 11 Oct 2016, Jakub Narębski wrote:

W dniu 11.10.2016 o 18:09, Johannes Schindelin pisze:

quoted

 SYNOPSIS
 --------
 [verse]
-'git reset' [-q] [<tree-ish>] [--] <paths>...
+'git reset' [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>...

I think you meant here

  +'git reset' [-q] [--stdin [-z]] [<tree-ish>]

Good point. I overlooked that the <paths>... are not optional here.

Thanks,
Dscho

Just like with other Git commands, this option makes it read the paths
from the standard input. It comes in handy when resetting many, many
paths at once and wildcards are not an option (e.g. when the paths are
generated by a tool).

Note: we first parse the entire list and perform the actual reset action
only in a second phase. Not only does this make things simpler, it also
helps performance, as do_diff_cache() traverses the index and the
(sorted) pathspecs in simultaneously to avoid unnecessary lookups.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-reset.txt |  9 ++++++++
 builtin/reset.c             | 54 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7107-reset-stdin.sh      | 33 +++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100755 t/t7107-reset-stdin.sh

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 25432d9257..abb71bb805 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt

@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [-q] [<tree-ish>] [--] <paths>...
 'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [--stdin [-z]] [<tree-ish>]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION

@@ -97,6 +98,14 @@ OPTIONS
 --quiet::
 	Be quiet, only report errors.
 
+--stdin::
+	Instead of taking list of paths from the command line,
+	read list of paths from the standard input.  Paths are
+	separated by LF (i.e. one path per line) by default.
+
+-z::
+	Only meaningful with `--stdin`; paths are separated with
+	NUL character instead of LF.
 
 EXAMPLES
 --------

diff --git a/builtin/reset.c b/builtin/reset.c
index 8ab915bfcb..1d3075b7ee 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c

@@ -21,10 +21,13 @@
 #include "parse-options.h"
 #include "unpack-trees.h"
 #include "cache-tree.h"
+#include "strbuf.h"
+#include "quote.h"
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+	N_("git reset [-q] [--stdin [-z]] [<tree-ish>]"),
 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
 	NULL
 };

@@ -267,7 +270,9 @@ static int reset_refs(const char *rev, const struct object_id *oid)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0, unborn;
+	int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
+	char **stdin_paths = NULL;
+	int stdin_nr = 0, stdin_alloc = 0;
 	const char *rev;
 	struct object_id oid;
 	struct pathspec pathspec;

@@ -286,6 +291,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
+		OPT_BOOL('z', NULL, &nul_term_line,
+			N_("paths are separated with NUL character")),
+		OPT_BOOL(0, "stdin", &read_from_stdin,
+				N_("read paths from <stdin>")),
 		OPT_END()
 	};
 

@@ -295,6 +304,43 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	if (read_from_stdin) {
+		strbuf_getline_fn getline_fn = nul_term_line ?
+			strbuf_getline_nul : strbuf_getline_lf;
+		int flags = PATHSPEC_PREFER_FULL |
+			PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
+		struct strbuf buf = STRBUF_INIT;
+		struct strbuf unquoted = STRBUF_INIT;
+
+		if (patch_mode)
+			die(_("--stdin is incompatible with --patch"));
+
+		if (pathspec.nr)
+			die(_("--stdin is incompatible with path arguments"));
+
+		while (getline_fn(&buf, stdin) != EOF) {
+			if (!nul_term_line && buf.buf[0] == '"') {
+				strbuf_reset(&unquoted);
+				if (unquote_c_style(&unquoted, buf.buf, NULL))
+					die(_("line is badly quoted"));
+				strbuf_swap(&buf, &unquoted);
+			}
+			ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+			stdin_paths[stdin_nr++] = xstrdup(buf.buf);
+			strbuf_reset(&buf);
+		}
+		strbuf_release(&unquoted);
+		strbuf_release(&buf);
+
+		ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+		stdin_paths[stdin_nr++] = NULL;
+		flags |= PATHSPEC_LITERAL_PATH;
+		parse_pathspec(&pathspec, 0, flags, prefix,
+			       (const char **)stdin_paths);
+
+	} else if (nul_term_line)
+		die(_("-z requires --stdin"));
+
 	unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", oid.hash);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */

@@ -385,5 +431,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (!pathspec.nr)
 		remove_branch_state();
 
+	if (stdin_paths) {
+		while (stdin_nr)
+			free(stdin_paths[--stdin_nr]);
+		free(stdin_paths);
+	}
+
 	return update_ref_status;
 }

diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh
new file mode 100755
index 0000000000..997dc42dd2
--- /dev/null
+++ b/t/t7107-reset-stdin.sh

@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='reset --stdin'
+
+. ./test-lib.sh
+
+test_expect_success 'reset --stdin' '
+	test_commit hello &&
+	git rm hello.t &&
+	test -z "$(git ls-files hello.t)" &&
+	echo hello.t | git reset --stdin &&
+	test hello.t = "$(git ls-files hello.t)"
+'
+
+test_expect_success 'reset --stdin -z' '
+	test_commit world &&
+	git rm hello.t world.t &&
+	test -z "$(git ls-files hello.t world.t)" &&
+	printf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&
+	printf "hello.t\nworld.t\n" >expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--stdin requires --mixed' '
+	echo hello.t >list &&
+	test_must_fail git reset --soft --stdin <list &&
+	test_must_fail git reset --hard --stdin <list &&
+	git reset --mixed --stdin <list
+'
+
+test_done
+

-- 
2.11.1.windows.prerelease.2.9.g3014b57

This feature was missing, and made it cumbersome for third-party
tools to reset a lot of paths in one go.

Support for --stdin has been added, following builtin/checkout-index.c's
example.

Changes since v1:

- adjusted commit message to explain why we read everything before
  resetting

- fixed synopsis (`--stdin` does not work with `-- <path>...`)

- avoid handling patch_mode twice

- use PATHSPEC_LITERAL_PATH to avoid interpreting input as wildcards


Johannes Schindelin (1):
  reset: support the --stdin option

 Documentation/git-reset.txt |  9 ++++++++
 builtin/reset.c             | 54 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7107-reset-stdin.sh      | 33 +++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100755 t/t7107-reset-stdin.sh


base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
Published-As: https://github.com/dscho/git/releases/tag/reset-stdin-v2
Fetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v2

Interdiff vs v1:

 diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
 index 533ef69f91..abb71bb805 100644
 --- a/Documentation/git-reset.txt
 +++ b/Documentation/git-reset.txt
 @@ -8,8 +8,9 @@ git-reset - Reset current HEAD to the specified state
  SYNOPSIS
  --------
  [verse]
 -'git reset' [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>...
 +'git reset' [-q] [<tree-ish>] [--] <paths>...
  'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
 +'git reset' [-q] [--stdin [-z]] [<tree-ish>]
  'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
  
  DESCRIPTION
 diff --git a/builtin/reset.c b/builtin/reset.c
 index 6de3936aed..1d3075b7ee 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -26,7 +26,8 @@
  
  static const char * const git_reset_usage[] = {
  	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 -	N_("git reset [-q] [--stdin [-z]] [<tree-ish>] [--] <paths>..."),
 +	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
 +	N_("git reset [-q] [--stdin [-z]] [<tree-ish>]"),
  	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
  	NULL
  };
 @@ -317,9 +318,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  		if (pathspec.nr)
  			die(_("--stdin is incompatible with path arguments"));
  
 -		if (patch_mode)
 -			flags |= PATHSPEC_PREFIX_ORIGIN;
 -
  		while (getline_fn(&buf, stdin) != EOF) {
  			if (!nul_term_line && buf.buf[0] == '"') {
  				strbuf_reset(&unquoted);
 @@ -336,8 +334,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  
  		ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
  		stdin_paths[stdin_nr++] = NULL;
 +		flags |= PATHSPEC_LITERAL_PATH;
  		parse_pathspec(&pathspec, 0, flags, prefix,
  			       (const char **)stdin_paths);
 +
  	} else if (nul_term_line)
  		die(_("-z requires --stdin"));
  

-- 
2.11.1.windows.prerelease.2.9.g3014b57

On Fri, Jan 27, 2017 at 01:38:55PM +0100, Johannes Schindelin wrote:

Just like with other Git commands, this option makes it read the paths
from the standard input. It comes in handy when resetting many, many
paths at once and wildcards are not an option (e.g. when the paths are
generated by a tool).

Note: we first parse the entire list and perform the actual reset action
only in a second phase. Not only does this make things simpler, it also
helps performance, as do_diff_cache() traverses the index and the
(sorted) pathspecs in simultaneously to avoid unnecessary lookups.

This looks OK to me. At first I wondered if using PATHSPEC_LITERAL_PATH
was consistent with other "--stdin" tools. I think it mostly is (or at
least consistent with checkout-index). The exception is "rev-list
--stdin", but that's probably fine. It is taking rev-list arguments in
the first place, not a list of paths.

A few minor suggestions:

Is it worth clarifying that these are paths, not pathspecs? The word
"paths" is used to refer to the pathspecs on the command-line elsewhere
in the document.

It might also be worth mentioning the quoting rules for the non-z case.

This list is a good candidate for an argv_array, I think. So:

  struct argv_array stdin_paths = ARGV_ARRAY_INIT;

+			ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+			stdin_paths[stdin_nr++] = xstrdup(buf.buf);

  argv_array_push(&stdin_paths, buf.buf);

+		ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
+		stdin_paths[stdin_nr++] = NULL;

  This goes away because argv_array handles it for you.

  argv_array_clear(&stdin_paths);

You set two flags here...

...and then one more here. They all seem to be set unconditionally, and
we never look at "flags" between the two lines. I think it would be more
obvious to set them all in the same place.

Hmm, there's our brace question coming up again. :)

Here are a few things we might want to test that I didn't see covered:

  - feeding path X does not reset path Y

  - de-quoting in the non-z case

-Peff

Hi Peff,

On Fri, 27 Jan 2017, Jeff King wrote:

On Fri, Jan 27, 2017 at 01:38:55PM +0100, Johannes Schindelin wrote:

A few minor suggestions:

quoted

+--stdin::
+	Instead of taking list of paths from the command line,
+	read list of paths from the standard input.  Paths are
+	separated by LF (i.e. one path per line) by default.
+
+-z::
+	Only meaningful with `--stdin`; paths are separated with
+	NUL character instead of LF.

Is it worth clarifying that these are paths, not pathspecs? The word
"paths" is used to refer to the pathspecs on the command-line elsewhere
in the document.

It might also be worth mentioning the quoting rules for the non-z case.

I think this would be overkill. In reality, --stdin without -z does not
make much sense, anyway.

If you feel strongly about it, I encourage you to submit a follow-up
patch.

The rest of your suggestions have been implemented in v3.

Ciao,
Johannes

Just like with other Git commands, this option makes it read the paths
from the standard input. It comes in handy when resetting many, many
paths at once and wildcards are not an option (e.g. when the paths are
generated by a tool).

Note: we first parse the entire list and perform the actual reset action
only in a second phase. Not only does this make things simpler, it also
helps performance, as do_diff_cache() traverses the index and the
(sorted) pathspecs in simultaneously to avoid unnecessary lookups.

Signed-off-by: Johannes Schindelin <redacted>
---
 Documentation/git-reset.txt | 10 ++++++++++
 builtin/reset.c             | 47 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7107-reset-stdin.sh      | 33 +++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100755 t/t7107-reset-stdin.sh

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 25432d9257..d319ed9b20 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt

@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git reset' [-q] [<tree-ish>] [--] <paths>...
 'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [--stdin [-z]] [<tree-ish>]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION

@@ -97,6 +98,15 @@ OPTIONS
 --quiet::
 	Be quiet, only report errors.
 
+--stdin::
+	Instead of taking list of paths from the command line,
+	read list of paths from the standard input.  The paths are
+	read verbatim, i.e. not handled as pathspecs.  Paths are
+	separated by LF (i.e. one path per line) by default.
+
+-z::
+	Only meaningful with `--stdin`; paths are separated with
+	NUL character instead of LF.
 
 EXAMPLES
 --------

diff --git a/builtin/reset.c b/builtin/reset.c
index 8ab915bfcb..fe7723c179 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c

@@ -21,10 +21,14 @@
 #include "parse-options.h"
 #include "unpack-trees.h"
 #include "cache-tree.h"
+#include "strbuf.h"
+#include "quote.h"
+#include "argv-array.h"
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+	N_("git reset [-q] [--stdin [-z]] [<tree-ish>]"),
 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
 	NULL
 };

@@ -267,7 +271,8 @@ static int reset_refs(const char *rev, const struct object_id *oid)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0, unborn;
+	int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
+	struct argv_array stdin_paths = ARGV_ARRAY_INIT;
 	const char *rev;
 	struct object_id oid;
 	struct pathspec pathspec;

@@ -286,6 +291,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
+		OPT_BOOL('z', NULL, &nul_term_line,
+			N_("paths are separated with NUL character")),
+		OPT_BOOL(0, "stdin", &read_from_stdin,
+				N_("read paths from <stdin>")),
 		OPT_END()
 	};
 

@@ -295,6 +304,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	if (read_from_stdin) {
+		strbuf_getline_fn getline_fn = nul_term_line ?
+			strbuf_getline_nul : strbuf_getline_lf;
+		int flags = PATHSPEC_PREFER_FULL |
+			PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;
+		struct strbuf buf = STRBUF_INIT;
+		struct strbuf unquoted = STRBUF_INIT;
+
+		if (patch_mode)
+			die(_("--stdin is incompatible with --patch"));
+
+		if (pathspec.nr)
+			die(_("--stdin is incompatible with path arguments"));
+
+		while (getline_fn(&buf, stdin) != EOF) {
+			if (!nul_term_line && buf.buf[0] == '"') {
+				strbuf_reset(&unquoted);
+				if (unquote_c_style(&unquoted, buf.buf, NULL))
+					die(_("line is badly quoted"));
+				strbuf_swap(&buf, &unquoted);
+			}
+			argv_array_push(&stdin_paths, buf.buf);
+			strbuf_reset(&buf);
+		}
+		strbuf_release(&unquoted);
+		strbuf_release(&buf);
+
+		flags |= PATHSPEC_LITERAL_PATH;
+		parse_pathspec(&pathspec, 0, flags, prefix,
+			       stdin_paths.argv);
+
+	} else if (nul_term_line)
+		die(_("-z requires --stdin"));
+
 	unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", oid.hash);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */

@@ -385,5 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	if (!pathspec.nr)
 		remove_branch_state();
 
+	argv_array_clear(&stdin_paths);
+
 	return update_ref_status;
 }

diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh
new file mode 100755
index 0000000000..997dc42dd2
--- /dev/null
+++ b/t/t7107-reset-stdin.sh

@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='reset --stdin'
+
+. ./test-lib.sh
+
+test_expect_success 'reset --stdin' '
+	test_commit hello &&
+	git rm hello.t &&
+	test -z "$(git ls-files hello.t)" &&
+	echo hello.t | git reset --stdin &&
+	test hello.t = "$(git ls-files hello.t)"
+'
+
+test_expect_success 'reset --stdin -z' '
+	test_commit world &&
+	git rm hello.t world.t &&
+	test -z "$(git ls-files hello.t world.t)" &&
+	printf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&
+	printf "hello.t\nworld.t\n" >expect &&
+	git ls-files >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success '--stdin requires --mixed' '
+	echo hello.t >list &&
+	test_must_fail git reset --soft --stdin <list &&
+	test_must_fail git reset --hard --stdin <list &&
+	git reset --mixed --stdin <list
+'
+
+test_done
+

-- 
2.11.1.windows.prerelease.2.9.g3014b57

This feature was missing, and made it cumbersome for third-party
tools to reset a lot of paths in one go.

Support for --stdin has been added, following builtin/checkout-index.c's
example.

Changes since v2:

- the documentation clarifies that --stdin does not treat the input as
  pathspecs

- the code now uses struct argv_array instead of rolling its own


Johannes Schindelin (1):
  reset: support the --stdin option

 Documentation/git-reset.txt | 10 ++++++++++
 builtin/reset.c             | 47 ++++++++++++++++++++++++++++++++++++++++++++-
 t/t7107-reset-stdin.sh      | 33 +++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100755 t/t7107-reset-stdin.sh


base-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe
Published-As: https://github.com/dscho/git/releases/tag/reset-stdin-v3
Fetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v3

Interdiff vs v2:

 diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
 index abb71bb805..d319ed9b20 100644
 --- a/Documentation/git-reset.txt
 +++ b/Documentation/git-reset.txt
 @@ -100,7 +100,8 @@ OPTIONS
  
  --stdin::
  	Instead of taking list of paths from the command line,
 -	read list of paths from the standard input.  Paths are
 +	read list of paths from the standard input.  The paths are
 +	read verbatim, i.e. not handled as pathspecs.  Paths are
  	separated by LF (i.e. one path per line) by default.
  
  -z::
 diff --git a/builtin/reset.c b/builtin/reset.c
 index 1d3075b7ee..fe7723c179 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -23,6 +23,7 @@
  #include "cache-tree.h"
  #include "strbuf.h"
  #include "quote.h"
 +#include "argv-array.h"
  
  static const char * const git_reset_usage[] = {
  	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 @@ -271,8 +272,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  {
  	int reset_type = NONE, update_ref_status = 0, quiet = 0;
  	int patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;
 -	char **stdin_paths = NULL;
 -	int stdin_nr = 0, stdin_alloc = 0;
 +	struct argv_array stdin_paths = ARGV_ARRAY_INIT;
  	const char *rev;
  	struct object_id oid;
  	struct pathspec pathspec;
 @@ -325,18 +325,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  					die(_("line is badly quoted"));
  				strbuf_swap(&buf, &unquoted);
  			}
 -			ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
 -			stdin_paths[stdin_nr++] = xstrdup(buf.buf);
 +			argv_array_push(&stdin_paths, buf.buf);
  			strbuf_reset(&buf);
  		}
  		strbuf_release(&unquoted);
  		strbuf_release(&buf);
  
 -		ALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);
 -		stdin_paths[stdin_nr++] = NULL;
  		flags |= PATHSPEC_LITERAL_PATH;
  		parse_pathspec(&pathspec, 0, flags, prefix,
 -			       (const char **)stdin_paths);
 +			       stdin_paths.argv);
  
  	} else if (nul_term_line)
  		die(_("-z requires --stdin"));
 @@ -431,11 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
  	if (!pathspec.nr)
  		remove_branch_state();
  
 -	if (stdin_paths) {
 -		while (stdin_nr)
 -			free(stdin_paths[--stdin_nr]);
 -		free(stdin_paths);
 -	}
 +	argv_array_clear(&stdin_paths);
  
  	return update_ref_status;
  }

-- 
2.11.1.windows.prerelease.2.9.g3014b57

On Fri, Jan 27, 2017 at 06:34:46PM +0100, Johannes Schindelin wrote:

quoted

Is it worth clarifying that these are paths, not pathspecs? The word
"paths" is used to refer to the pathspecs on the command-line elsewhere
in the document.

It might also be worth mentioning the quoting rules for the non-z case.

I think this would be overkill. In reality, --stdin without -z does not
make much sense, anyway.

I think with most Unix tools people tend to use the non-z forms until
they break, and then switch to the z form. :)

And in that sense, this transparently Just Works because the output will
often come from another git tool, which will quote as appropriate.

If you feel strongly about it, I encourage you to submit a follow-up
patch.

The rest of your suggestions have been implemented in v3.

Thanks. I think the path/pathspec thing was the more important of the
two suggestions.

-Peff