{"thread":{"id":"44275","subject":"[PATCH 1/2] reset: fix usage","startedAt":"2016-10-11T16:15:52Z","lastAt":"2017-01-28T00:20:45Z","messageCount":23,"participants":["Johannes Schindelin","Junio C Hamano","Jeff King","Jakub Narębski"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"303879","messageId":"6b6defb4164b07f4722c7e3cfa21648f7d4c0e87.1476202100.git.johannes.schindelin@gmx.de","threadId":"44275","inReplyTo":"cover.1476202100.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/2] reset: fix usage","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-11T16:09:03Z","receivedAt":"2016-10-11T16:15:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The parameter is actually optional (see man page).\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/reset.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 5aa8607..c04ac07 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -24,7 +24,7 @@\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n-\tN_(\"git reset [-q] [--] ...\"),\n+\tN_(\"git reset [-q] [] [--] ...\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n-- \n2.10.1.513.g00ef6dd\n\n\n"},{"id":"303880","messageId":"cover.1476202100.git.johannes.schindelin@gmx.de","threadId":"44275","inReplyTo":null,"subject":"[PATCH 0/2] Support `git reset --stdin`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-11T16:08:56Z","receivedAt":"2016-10-11T16:16:28Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"This feature was missing, and made it cumbersome for third-party\ntools to reset a lot of paths in one go.\n\nSupport for --stdin has been added, following builtin/checkout-index.c's\nexample.\n\n\nJohannes Schindelin (2):\n reset: fix usage\n reset: support the --stdin option\n\n Documentation/git-reset.txt | 10 +++++++-\n builtin/reset.c | 56 +++++++++++++++++++++++++++++++++++++++++++--\n t/t7107-reset-stdin.sh | 33 ++++++++++++++++++++++++++\n 3 files changed, 96 insertions(+), 3 deletions(-)\n create mode 100755 t/t7107-reset-stdin.sh\n\n\nbase-commit: 8a36cd87b7c85a651ab388d403629865ffa3ba0d\nPublished-As: https://github.com/dscho/git/releases/tag/reset-stdin-v1\nFetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v1\n\n-- \n2.10.1.513.g00ef6dd\n\n"},{"id":"303881","messageId":"2c7a52e43be710c7f37c4886629bda38df183c21.1476202100.git.johannes.schindelin@gmx.de","threadId":"44275","inReplyTo":"cover.1476202100.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/2] reset: support the --stdin option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-11T16:09:11Z","receivedAt":"2016-10-11T16:19:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Just like with other Git commands, this option makes it read the paths\nfrom the standard input. It comes in handy when resetting many, many\npaths at once and wildcards are not an option (e.g. when the paths are\ngenerated by a tool).\n\nNote: to keep things simple, we first parse the entire list and perform\nthe actual reset action only in a second phase.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-reset.txt | 10 +++++++-\n builtin/reset.c | 56 +++++++++++++++++++++++++++++++++++++++++++--\n t/t7107-reset-stdin.sh | 33 ++++++++++++++++++++++++++\n 3 files changed, 96 insertions(+), 3 deletions(-)\n create mode 100755 t/t7107-reset-stdin.sh\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 25432d9..533ef69 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -8,7 +8,7 @@ git-reset - Reset current HEAD to the specified state\n SYNOPSIS\n --------\n [verse]\n-'git reset' [-q] [] [--] ...\n+'git reset' [-q] [--stdin [-z]] [] [--] ...\n 'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n@@ -97,6 +97,14 @@ OPTIONS\n --quiet::\n \tBe quiet, only report errors.\n \n+--stdin::\n+\tInstead of taking list of paths from the command line,\n+\tread list of paths from the standard input. Paths are\n+\tseparated by LF (i.e. one path per line) by default.\n+\n+-z::\n+\tOnly meaningful with `--stdin`; paths are separated with\n+\tNUL character instead of LF.\n \n EXAMPLES\n --------\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex c04ac07..018735f 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -21,10 +21,12 @@\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n #include \"cache-tree.h\"\n+#include \"strbuf.h\"\n+#include \"quote.h\"\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n-\tN_(\"git reset [-q] [] [--] ...\"),\n+\tN_(\"git reset [-q] [--stdin [-z]] [] [--] ...\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n@@ -267,7 +269,9 @@ static int reset_refs(const char *rev, const struct object_id *oid)\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint patch_mode = 0, unborn;\n+\tint patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;\n+\tchar **stdin_paths = NULL;\n+\tint stdin_nr = 0, stdin_alloc = 0;\n \tconst char *rev;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n@@ -286,6 +290,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n \t\tOPT_BOOL('N', \"intent-to-add\", &intent_to_add,\n \t\t\t\tN_(\"record only the fact that removed paths will be added later\")),\n+\t\tOPT_BOOL('z', NULL, &nul_term_line,\n+\t\t\tN_(\"paths are separated with NUL character\")),\n+\t\tOPT_BOOL(0, \"stdin\", &read_from_stdin,\n+\t\t\t\tN_(\"read paths from \")),\n \t\tOPT_END()\n \t};\n \n@@ -295,6 +303,44 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \n+\tif (read_from_stdin) {\n+\t\tstrbuf_getline_fn getline_fn = nul_term_line ?\n+\t\t\tstrbuf_getline_nul : strbuf_getline_lf;\n+\t\tint flags = PATHSPEC_PREFER_FULL |\n+\t\t\tPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstruct strbuf unquoted = STRBUF_INIT;\n+\n+\t\tif (patch_mode)\n+\t\t\tdie(_(\"--stdin is incompatible with --patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--stdin is incompatible with path arguments\"));\n+\n+\t\tif (patch_mode)\n+\t\t\tflags |= PATHSPEC_PREFIX_ORIGIN;\n+\n+\t\twhile (getline_fn(&buf, stdin) != EOF) {\n+\t\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n+\t\t\t\tstrbuf_reset(&unquoted);\n+\t\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n+\t\t\t\t\tdie(_(\"line is badly quoted\"));\n+\t\t\t\tstrbuf_swap(&buf, &unquoted);\n+\t\t\t}\n+\t\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n+\t\t\tstdin_paths[stdin_nr++] = xstrdup(buf.buf);\n+\t\t\tstrbuf_reset(&buf);\n+\t\t}\n+\t\tstrbuf_release(&unquoted);\n+\t\tstrbuf_release(&buf);\n+\n+\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n+\t\tstdin_paths[stdin_nr++] = NULL;\n+\t\tparse_pathspec(&pathspec, 0, flags, prefix,\n+\t\t\t (const char **)stdin_paths);\n+\t} else if (nul_term_line)\n+\t\tdie(_(\"-z requires --stdin\"));\n+\n \tunborn = !strcmp(rev, \"HEAD\") && get_sha1(\"HEAD\", oid.hash);\n \tif (unborn) {\n \t\t/* reset on unborn branch: treat as reset to empty tree */\n@@ -385,5 +431,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (!pathspec.nr)\n \t\tremove_branch_state();\n \n+\tif (stdin_paths) {\n+\t\twhile (stdin_nr)\n+\t\t\tfree(stdin_paths[--stdin_nr]);\n+\t\tfree(stdin_paths);\n+\t}\n+\n \treturn update_ref_status;\n }\ndiff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh\nnew file mode 100755\nindex 0000000..997dc42\n--- /dev/null\n+++ b/t/t7107-reset-stdin.sh\n@@ -0,0 +1,33 @@\n+#!/bin/sh\n+\n+test_description='reset --stdin'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'reset --stdin' '\n+\ttest_commit hello &&\n+\tgit rm hello.t &&\n+\ttest -z \"$(git ls-files hello.t)\" &&\n+\techo hello.t | git reset --stdin &&\n+\ttest hello.t = \"$(git ls-files hello.t)\"\n+'\n+\n+test_expect_success 'reset --stdin -z' '\n+\ttest_commit world &&\n+\tgit rm hello.t world.t &&\n+\ttest -z \"$(git ls-files hello.t world.t)\" &&\n+\tprintf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&\n+\tprintf \"hello.t\\nworld.t\\n\" >expect &&\n+\tgit ls-files >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--stdin requires --mixed' '\n+\techo hello.t >list &&\n+\ttest_must_fail git reset --soft --stdin writes:\n\n> +\tif (read_from_stdin) {\n> +\t\tstrbuf_getline_fn getline_fn = nul_term_line ?\n> +\t\t\tstrbuf_getline_nul : strbuf_getline_lf;\n> +\t\tint flags = PATHSPEC_PREFER_FULL |\n> +\t\t\tPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\t\tstruct strbuf unquoted = STRBUF_INIT;\n> +\n> +\t\tif (patch_mode)\n> +\t\t\tdie(_(\"--stdin is incompatible with --patch\"));\n> +\n> +\t\tif (pathspec.nr)\n> +\t\t\tdie(_(\"--stdin is incompatible with path arguments\"));\n> +\n> +\t\tif (patch_mode)\n> +\t\t\tflags |= PATHSPEC_PREFIX_ORIGIN;\n\nDidn't we already die above under that mode?\n\n> +\t\twhile (getline_fn(&buf, stdin) != EOF) {\n> +\t\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n> +\t\t\t\tstrbuf_reset(&unquoted);\n> +\t\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n> +\t\t\t\t\tdie(_(\"line is badly quoted\"));\n> +\t\t\t\tstrbuf_swap(&buf, &unquoted);\n> +\t\t\t}\n> +\t\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n> +\t\t\tstdin_paths[stdin_nr++] = xstrdup(buf.buf);\n> +\t\t\tstrbuf_reset(&buf);\n> +\t\t}\n> +\t\tstrbuf_release(&unquoted);\n> +\t\tstrbuf_release(&buf);\n> +\n> +\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n> +\t\tstdin_paths[stdin_nr++] = NULL;\n\nIt makes sense to collect, but...\n\n> +\t\tparse_pathspec(&pathspec, 0, flags, prefix,\n> +\t\t\t (const char **)stdin_paths);\n\n...letting them be used as if they are pathspec is wrong when\nstdin_paths[] contain wildcard, isn't it? \n\nI think flags |= PATHSPEC_LITERAL_PATH can help fixing it. 0/2 said\nthis mimicks checkout-index and I think it should by not treating\nthe input as wildcarded patterns (i.e. \"echo '*.c' | reset --stdin\"\nshouldn't be the way to reset all .c files --- that's something we\nwould want to add to the test, I guess).\n\nThanks.\n\n"},{"id":"303907","messageId":"20161011183448.yasglfjelo4kgrq4@sigill.intra.peff.net","threadId":"44275","inReplyTo":"cover.1476202100.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 0/2] Support `git reset --stdin`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-11T18:34:49Z","receivedAt":"2016-10-11T19:03:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 11, 2016 at 06:08:56PM +0200, Johannes Schindelin wrote:\n\n> This feature was missing, and made it cumbersome for third-party\n> tools to reset a lot of paths in one go.\n> \n> Support for --stdin has been added, following builtin/checkout-index.c's\n> example.\n\nIs git-reset the right layer to add scripting features? I thought we\nusually pushed people doing mass index manipulation to use update-index\nor read-tree. Is there something that reset makes easy that is hard with\nthose tools (I could imagine \"--hard\", but I see it is not supported\nwith your patch).\n\nNot that I'm necessarily opposed to the patch, I was just surprised.\n\n-Peff\n"},{"id":"303915","messageId":"xmqqshs28z52.fsf@gitster.mtv.corp.google.com","threadId":"44275","inReplyTo":"20161011183448.yasglfjelo4kgrq4@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Support `git reset --stdin`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-11T19:27:21Z","receivedAt":"2016-10-11T19:40:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Is git-reset the right layer to add scripting features? I thought we\n> usually pushed people doing mass index manipulation to use update-index\n> or read-tree. Is there something that reset makes easy that is hard with\n> those tools (I could imagine \"--hard\", but I see it is not supported\n> with your patch).\n>\n> Not that I'm necessarily opposed to the patch, I was just surprised.\n\nIf read-tree had pathspec support (i.e. \"read these and only these\npaths given from the command line into the index from a given\ntree-ish\"), that would have been the most natural place to extend\nwith \"oh, by the way, instead of the command line, you can feed the\npaths on the standard input\". \n\nBut it doesn't have one.\n\n"},{"id":"303927","messageId":"dc76476b-9ad5-a3b6-f12f-33cda2ca5814@gmail.com","threadId":"44275","inReplyTo":"2c7a52e43be710c7f37c4886629bda38df183c21.1476202100.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/2] reset: support the --stdin option","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-10-11T20:49:54Z","receivedAt":"2016-10-11T21:04:56Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 11.10.2016 o 18:09, Johannes Schindelin pisze:\n\n> SYNOPSIS\n> --------\n> [verse]\n> -'git reset' [-q] [] [--] ...\n> +'git reset' [-q] [--stdin [-z]] [] [--] ...\n\nI think you meant here\n\n +'git reset' [-q] [--stdin [-z]] []\n\nBecause you say \"*Instead*\" below.\n\n> +--stdin::\n> +\tInstead of taking list of paths from the command line,\n> +\tread list of paths from the standard input. Paths are\n> +\tseparated by LF (i.e. one path per line) by default.\n\nAnd die if were supplied:\n\n> +\t\tif (pathspec.nr)\n> +\t\t\tdie(_(\"--stdin is incompatible with path arguments\"));\n\nOf course you need to fix it in built-in synopsis as well:\n\n> +\tN_(\"git reset [-q] [--stdin [-z]] [] [--] ...\"),\n> \tN_(\"git reset --patch [] [--] [...]\"),\n\n-- \nJakub Narębski\n\n"},{"id":"303931","messageId":"20161011212644.zzqidtcgatu3qsei@sigill.intra.peff.net","threadId":"44275","inReplyTo":"xmqqshs28z52.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/2] Support `git reset --stdin`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-11T21:26:44Z","receivedAt":"2016-10-11T21:26:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 11, 2016 at 12:27:21PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Is git-reset the right layer to add scripting features? I thought we\n> > usually pushed people doing mass index manipulation to use update-index\n> > or read-tree. Is there something that reset makes easy that is hard with\n> > those tools (I could imagine \"--hard\", but I see it is not supported\n> > with your patch).\n> >\n> > Not that I'm necessarily opposed to the patch, I was just surprised.\n> \n> If read-tree had pathspec support (i.e. \"read these and only these\n> paths given from the command line into the index from a given\n> tree-ish\"), that would have been the most natural place to extend\n> with \"oh, by the way, instead of the command line, you can feed the\n> paths on the standard input\".\n> \n> But it doesn't have one.\n\nTrue. I'd have done something more like:\n\n git ls-tree -r $paths | git update-index --index-info\n\nbut there are some corner cases around deleting paths from the index.\n\n-Peff\n"},{"id":"303933","messageId":"xmqqoa2q7elc.fsf@gitster.mtv.corp.google.com","threadId":"44275","inReplyTo":"20161011212644.zzqidtcgatu3qsei@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Support `git reset --stdin`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-11T21:36:31Z","receivedAt":"2016-10-11T21:36:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> If read-tree had pathspec support (i.e. \"read these and only these\n>> paths given from the command line into the index from a given\n>> tree-ish\"), that would have been the most natural place to extend\n>> with \"oh, by the way, instead of the command line, you can feed the\n>> paths on the standard input\".\n>> \n>> But it doesn't have one.\n>\n> True. I'd have done something more like:\n>\n> git ls-tree -r $paths | git update-index --index-info\n>\n> but there are some corner cases around deleting paths from the index.\n\nAh, I would think read-tree has the exact same issue, even if we\nadded pathspec support, around removal.\n\nSo it is more like\n\n\t(\n\t\tprintf \"0 0000000000000000000000000000000000000000\\t%s\\n\" $paths\n\t\tgit --literal-pathspecs ls-tree -r --ignore-missing $paths\n\t) | git update-index --index-info\n\nwhich does not look too bad, even though this\n\n\tprintf \"%s\\n\" $paths | git reset --stdin\n\ndoes look shorter.\n\n"},{"id":"303936","messageId":"20161011214713.y2fpjkrx6sspks4a@sigill.intra.peff.net","threadId":"44275","inReplyTo":"xmqqoa2q7elc.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/2] Support `git reset --stdin`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-10-11T21:47:14Z","receivedAt":"2016-10-11T21:48:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 11, 2016 at 02:36:31PM -0700, Junio C Hamano wrote:\n\n> > True. I'd have done something more like:\n> >\n> > git ls-tree -r $paths | git update-index --index-info\n> >\n> > but there are some corner cases around deleting paths from the index.\n> \n> Ah, I would think read-tree has the exact same issue, even if we\n> added pathspec support, around removal.\n> \n> So it is more like\n> \n> \t(\n> \t\tprintf \"0 0000000000000000000000000000000000000000\\t%s\\n\" $paths\n> \t\tgit --literal-pathspecs ls-tree -r --ignore-missing $paths\n> \t) | git update-index --index-info\n> \n> which does not look too bad, even though this\n> \n> \tprintf \"%s\\n\" $paths | git reset --stdin\n> \n> does look shorter.\n\nOf course neither of ours solutions works when \"$paths\" is coming on\nstdin, rather than in a variable, which I suspect was Dscho's original\nmotivation. :)\n\nOne reason not to do the unconditional $z40 in yours is that without it,\nI would hope that update-index is smart enough not to discard the stat\ninformation for entries which are unchanged.\n\nI suspect the best answer is more like:\n\n git diff-index --cached HEAD | git update-index --index-info\n\nexcept that you have to munge the data in between, because update-index\ndoes not know how to pick the correct data out of the --raw diff output.\nBut that's probably closer to what git-reset does internally.\n\nAnyway, the existence of this discussion is probably a good argument in\nfavor of Dscho's patch. I was mostly curious how close our plumbing\ntools could come.\n\n-Peff\n"},{"id":"303937","messageId":"xmqqfuo27dzj.fsf@gitster.mtv.corp.google.com","threadId":"44275","inReplyTo":"20161011214713.y2fpjkrx6sspks4a@sigill.intra.peff.net","subject":"Re: [PATCH 0/2] Support `git reset --stdin`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-11T21:49:36Z","receivedAt":"2016-10-11T21:49:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Anyway, the existence of this discussion is probably a good argument in\n> favor of Dscho's patch. I was mostly curious how close our plumbing\n> tools could come.\n\nSure, in 100% agreement.\n"},{"id":"303980","messageId":"alpine.DEB.2.20.1610121438400.3492@virtualbox","threadId":"44275","inReplyTo":"dc76476b-9ad5-a3b6-f12f-33cda2ca5814@gmail.com","subject":"Re: [PATCH 2/2] reset: support the --stdin option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-12T12:39:05Z","receivedAt":"2016-10-12T12:46:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Kuba,\n\nOn Tue, 11 Oct 2016, Jakub Narębski wrote:\n\n> W dniu 11.10.2016 o 18:09, Johannes Schindelin pisze:\n> \n> > SYNOPSIS\n> > --------\n> > [verse]\n> > -'git reset' [-q] [] [--] ...\n> > +'git reset' [-q] [--stdin [-z]] [] [--] ...\n> \n> I think you meant here\n> \n> +'git reset' [-q] [--stdin [-z]] []\n\nGood point. I overlooked that the ... are not optional here.\n\nThanks,\nDscho"},{"id":"303981","messageId":"alpine.DEB.2.20.1610121449190.3492@virtualbox","threadId":"44275","inReplyTo":"xmqqeg3mai1b.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] reset: support the --stdin option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-10-12T12:57:05Z","receivedAt":"2016-10-12T12:58:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 11 Oct 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > +\tif (read_from_stdin) {\n> > +\t\tstrbuf_getline_fn getline_fn = nul_term_line ?\n> > +\t\t\tstrbuf_getline_nul : strbuf_getline_lf;\n> > +\t\tint flags = PATHSPEC_PREFER_FULL |\n> > +\t\t\tPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;\n> > +\t\tstruct strbuf buf = STRBUF_INIT;\n> > +\t\tstruct strbuf unquoted = STRBUF_INIT;\n> > +\n> > +\t\tif (patch_mode)\n> > +\t\t\tdie(_(\"--stdin is incompatible with --patch\"));\n> > +\n> > +\t\tif (pathspec.nr)\n> > +\t\t\tdie(_(\"--stdin is incompatible with path arguments\"));\n> > +\n> > +\t\tif (patch_mode)\n> > +\t\t\tflags |= PATHSPEC_PREFIX_ORIGIN;\n> \n> Didn't we already die above under that mode?\n\nOh right. Copy/paste fail.\n\n> > +\t\twhile (getline_fn(&buf, stdin) != EOF) {\n> > +\t\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n> > +\t\t\t\tstrbuf_reset(&unquoted);\n> > +\t\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n> > +\t\t\t\t\tdie(_(\"line is badly quoted\"));\n> > +\t\t\t\tstrbuf_swap(&buf, &unquoted);\n> > +\t\t\t}\n> > +\t\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n> > +\t\t\tstdin_paths[stdin_nr++] = xstrdup(buf.buf);\n> > +\t\t\tstrbuf_reset(&buf);\n> > +\t\t}\n> > +\t\tstrbuf_release(&unquoted);\n> > +\t\tstrbuf_release(&buf);\n> > +\n> > +\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n> > +\t\tstdin_paths[stdin_nr++] = NULL;\n> \n> It makes sense to collect, but...\n\nIt does, doesn't it? I really would have loved to start resetting right\naway, but if the list were not sorted and traversed at the same time as\nthe tree-ish, the performance would just be suboptimal.\n\nI think that is an important point and I adjusted the commit message\naccordingly.\n\n> > +\t\tparse_pathspec(&pathspec, 0, flags, prefix,\n> > +\t\t\t (const char **)stdin_paths);\n> \n> ...letting them be used as if they are pathspec is wrong when\n> stdin_paths[] contain wildcard, isn't it? \n> \n> I think flags |= PATHSPEC_LITERAL_PATH can help fixing it. 0/2 said\n> this mimicks checkout-index and I think it should by not treating\n> the input as wildcarded patterns (i.e. \"echo '*.c' | reset --stdin\"\n> shouldn't be the way to reset all .c files --- that's something we\n> would want to add to the test, I guess).\n\nTrue. I adjust the flags accordingly now.\n\nThanks,\nDscho\n"},{"id":"310404","messageId":"3b0bad26045ed175f40f050e15b65cb6a8f2368c.1485520718.git.johannes.schindelin@gmx.de","threadId":"44275","inReplyTo":"cover.1485520718.git.johannes.schindelin@gmx.de","subject":"[PATCH v2 1/1] reset: support the --stdin option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-27T12:38:55Z","receivedAt":"2017-01-27T12:56:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Just like with other Git commands, this option makes it read the paths\nfrom the standard input. It comes in handy when resetting many, many\npaths at once and wildcards are not an option (e.g. when the paths are\ngenerated by a tool).\n\nNote: we first parse the entire list and perform the actual reset action\nonly in a second phase. Not only does this make things simpler, it also\nhelps performance, as do_diff_cache() traverses the index and the\n(sorted) pathspecs in simultaneously to avoid unnecessary lookups.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-reset.txt | 9 ++++++++\n builtin/reset.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-\n t/t7107-reset-stdin.sh | 33 +++++++++++++++++++++++++++\n 3 files changed, 95 insertions(+), 1 deletion(-)\n create mode 100755 t/t7107-reset-stdin.sh\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 25432d9257..abb71bb805 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -10,6 +10,7 @@ SYNOPSIS\n [verse]\n 'git reset' [-q] [] [--] ...\n 'git reset' (--patch | -p) [] [--] [...]\n+'git reset' [-q] [--stdin [-z]] []\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n@@ -97,6 +98,14 @@ OPTIONS\n --quiet::\n \tBe quiet, only report errors.\n \n+--stdin::\n+\tInstead of taking list of paths from the command line,\n+\tread list of paths from the standard input. Paths are\n+\tseparated by LF (i.e. one path per line) by default.\n+\n+-z::\n+\tOnly meaningful with `--stdin`; paths are separated with\n+\tNUL character instead of LF.\n \n EXAMPLES\n --------\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8ab915bfcb..1d3075b7ee 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -21,10 +21,13 @@\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n #include \"cache-tree.h\"\n+#include \"strbuf.h\"\n+#include \"quote.h\"\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n \tN_(\"git reset [-q] [] [--] ...\"),\n+\tN_(\"git reset [-q] [--stdin [-z]] []\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n@@ -267,7 +270,9 @@ static int reset_refs(const char *rev, const struct object_id *oid)\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint patch_mode = 0, unborn;\n+\tint patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;\n+\tchar **stdin_paths = NULL;\n+\tint stdin_nr = 0, stdin_alloc = 0;\n \tconst char *rev;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n@@ -286,6 +291,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n \t\tOPT_BOOL('N', \"intent-to-add\", &intent_to_add,\n \t\t\t\tN_(\"record only the fact that removed paths will be added later\")),\n+\t\tOPT_BOOL('z', NULL, &nul_term_line,\n+\t\t\tN_(\"paths are separated with NUL character\")),\n+\t\tOPT_BOOL(0, \"stdin\", &read_from_stdin,\n+\t\t\t\tN_(\"read paths from \")),\n \t\tOPT_END()\n \t};\n \n@@ -295,6 +304,43 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \n+\tif (read_from_stdin) {\n+\t\tstrbuf_getline_fn getline_fn = nul_term_line ?\n+\t\t\tstrbuf_getline_nul : strbuf_getline_lf;\n+\t\tint flags = PATHSPEC_PREFER_FULL |\n+\t\t\tPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstruct strbuf unquoted = STRBUF_INIT;\n+\n+\t\tif (patch_mode)\n+\t\t\tdie(_(\"--stdin is incompatible with --patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--stdin is incompatible with path arguments\"));\n+\n+\t\twhile (getline_fn(&buf, stdin) != EOF) {\n+\t\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n+\t\t\t\tstrbuf_reset(&unquoted);\n+\t\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n+\t\t\t\t\tdie(_(\"line is badly quoted\"));\n+\t\t\t\tstrbuf_swap(&buf, &unquoted);\n+\t\t\t}\n+\t\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n+\t\t\tstdin_paths[stdin_nr++] = xstrdup(buf.buf);\n+\t\t\tstrbuf_reset(&buf);\n+\t\t}\n+\t\tstrbuf_release(&unquoted);\n+\t\tstrbuf_release(&buf);\n+\n+\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n+\t\tstdin_paths[stdin_nr++] = NULL;\n+\t\tflags |= PATHSPEC_LITERAL_PATH;\n+\t\tparse_pathspec(&pathspec, 0, flags, prefix,\n+\t\t\t (const char **)stdin_paths);\n+\n+\t} else if (nul_term_line)\n+\t\tdie(_(\"-z requires --stdin\"));\n+\n \tunborn = !strcmp(rev, \"HEAD\") && get_sha1(\"HEAD\", oid.hash);\n \tif (unborn) {\n \t\t/* reset on unborn branch: treat as reset to empty tree */\n@@ -385,5 +431,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (!pathspec.nr)\n \t\tremove_branch_state();\n \n+\tif (stdin_paths) {\n+\t\twhile (stdin_nr)\n+\t\t\tfree(stdin_paths[--stdin_nr]);\n+\t\tfree(stdin_paths);\n+\t}\n+\n \treturn update_ref_status;\n }\ndiff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh\nnew file mode 100755\nindex 0000000000..997dc42dd2\n--- /dev/null\n+++ b/t/t7107-reset-stdin.sh\n@@ -0,0 +1,33 @@\n+#!/bin/sh\n+\n+test_description='reset --stdin'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'reset --stdin' '\n+\ttest_commit hello &&\n+\tgit rm hello.t &&\n+\ttest -z \"$(git ls-files hello.t)\" &&\n+\techo hello.t | git reset --stdin &&\n+\ttest hello.t = \"$(git ls-files hello.t)\"\n+'\n+\n+test_expect_success 'reset --stdin -z' '\n+\ttest_commit world &&\n+\tgit rm hello.t world.t &&\n+\ttest -z \"$(git ls-files hello.t world.t)\" &&\n+\tprintf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&\n+\tprintf \"hello.t\\nworld.t\\n\" >expect &&\n+\tgit ls-files >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--stdin requires --mixed' '\n+\techo hello.t >list &&\n+\ttest_must_fail git reset --soft --stdin ...`)\n\n- avoid handling patch_mode twice\n\n- use PATHSPEC_LITERAL_PATH to avoid interpreting input as wildcards\n\n\nJohannes Schindelin (1):\n reset: support the --stdin option\n\n Documentation/git-reset.txt | 9 ++++++++\n builtin/reset.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-\n t/t7107-reset-stdin.sh | 33 +++++++++++++++++++++++++++\n 3 files changed, 95 insertions(+), 1 deletion(-)\n create mode 100755 t/t7107-reset-stdin.sh\n\n\nbase-commit: 4e59582ff70d299f5a88449891e78d15b4b3fabe\nPublished-As: https://github.com/dscho/git/releases/tag/reset-stdin-v2\nFetch-It-Via: git fetch https://github.com/dscho/git reset-stdin-v2\n\nInterdiff vs v1:\n\n diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n index 533ef69f91..abb71bb805 100644\n --- a/Documentation/git-reset.txt\n +++ b/Documentation/git-reset.txt\n @@ -8,8 +8,9 @@ git-reset - Reset current HEAD to the specified state\n SYNOPSIS\n --------\n [verse]\n -'git reset' [-q] [--stdin [-z]] [] [--] ...\n +'git reset' [-q] [] [--] ...\n 'git reset' (--patch | -p) [] [--] [...]\n +'git reset' [-q] [--stdin [-z]] []\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n diff --git a/builtin/reset.c b/builtin/reset.c\n index 6de3936aed..1d3075b7ee 100644\n --- a/builtin/reset.c\n +++ b/builtin/reset.c\n @@ -26,7 +26,8 @@\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n -\tN_(\"git reset [-q] [--stdin [-z]] [] [--] ...\"),\n +\tN_(\"git reset [-q] [] [--] ...\"),\n +\tN_(\"git reset [-q] [--stdin [-z]] []\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n @@ -317,9 +318,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tif (pathspec.nr)\n \t\t\tdie(_(\"--stdin is incompatible with path arguments\"));\n \n -\t\tif (patch_mode)\n -\t\t\tflags |= PATHSPEC_PREFIX_ORIGIN;\n -\n \t\twhile (getline_fn(&buf, stdin) != EOF) {\n \t\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n \t\t\t\tstrbuf_reset(&unquoted);\n @@ -336,8 +334,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \n \t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n \t\tstdin_paths[stdin_nr++] = NULL;\n +\t\tflags |= PATHSPEC_LITERAL_PATH;\n \t\tparse_pathspec(&pathspec, 0, flags, prefix,\n \t\t\t (const char **)stdin_paths);\n +\n \t} else if (nul_term_line)\n \t\tdie(_(\"-z requires --stdin\"));\n \n\n-- \n2.11.1.windows.prerelease.2.9.g3014b57\n\n"},{"id":"310410","messageId":"20170127170422.lvpghp6jdud37zxx@sigill.intra.peff.net","threadId":"44275","inReplyTo":"3b0bad26045ed175f40f050e15b65cb6a8f2368c.1485520718.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH v2 1/1] reset: support the --stdin option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-27T17:04:22Z","receivedAt":"2017-01-27T17:11:42Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2017 at 01:38:55PM +0100, Johannes Schindelin wrote:\n\n> Just like with other Git commands, this option makes it read the paths\n> from the standard input. It comes in handy when resetting many, many\n> paths at once and wildcards are not an option (e.g. when the paths are\n> generated by a tool).\n> \n> Note: we first parse the entire list and perform the actual reset action\n> only in a second phase. Not only does this make things simpler, it also\n> helps performance, as do_diff_cache() traverses the index and the\n> (sorted) pathspecs in simultaneously to avoid unnecessary lookups.\n\nThis looks OK to me. At first I wondered if using PATHSPEC_LITERAL_PATH\nwas consistent with other \"--stdin\" tools. I think it mostly is (or at\nleast consistent with checkout-index). The exception is \"rev-list\n--stdin\", but that's probably fine. It is taking rev-list arguments in\nthe first place, not a list of paths.\n\nA few minor suggestions:\n\n> +--stdin::\n> +\tInstead of taking list of paths from the command line,\n> +\tread list of paths from the standard input. Paths are\n> +\tseparated by LF (i.e. one path per line) by default.\n> +\n> +-z::\n> +\tOnly meaningful with `--stdin`; paths are separated with\n> +\tNUL character instead of LF.\n\nIs it worth clarifying that these are paths, not pathspecs? The word\n\"paths\" is used to refer to the pathspecs on the command-line elsewhere\nin the document.\n\nIt might also be worth mentioning the quoting rules for the non-z case.\n\n> @@ -267,7 +270,9 @@ static int reset_refs(const char *rev, const struct object_id *oid)\n> int cmd_reset(int argc, const char **argv, const char *prefix)\n> {\n> \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n> -\tint patch_mode = 0, unborn;\n> +\tint patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;\n> +\tchar **stdin_paths = NULL;\n> +\tint stdin_nr = 0, stdin_alloc = 0;\n\nThis list is a good candidate for an argv_array, I think. So:\n\n struct argv_array stdin_paths = ARGV_ARRAY_INIT;\n\n> +\t\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n> +\t\t\tstdin_paths[stdin_nr++] = xstrdup(buf.buf);\n\n argv_array_push(&stdin_paths, buf.buf);\n\n> +\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n> +\t\tstdin_paths[stdin_nr++] = NULL;\n\n This goes away because argv_array handles it for you.\n\n> +\tif (stdin_paths) {\n> +\t\twhile (stdin_nr)\n> +\t\t\tfree(stdin_paths[--stdin_nr]);\n> +\t\tfree(stdin_paths);\n> +\t}\n\n argv_array_clear(&stdin_paths);\n\n> @@ -295,6 +304,43 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n> \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n> \n> +\tif (read_from_stdin) {\n> +\t\tstrbuf_getline_fn getline_fn = nul_term_line ?\n> +\t\t\tstrbuf_getline_nul : strbuf_getline_lf;\n> +\t\tint flags = PATHSPEC_PREFER_FULL |\n> +\t\t\tPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;\n\nYou set two flags here...\n\n> +\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n> +\t\tstdin_paths[stdin_nr++] = NULL;\n> +\t\tflags |= PATHSPEC_LITERAL_PATH;\n> +\t\tparse_pathspec(&pathspec, 0, flags, prefix,\n> +\t\t\t (const char **)stdin_paths);\n\n...and then one more here. They all seem to be set unconditionally, and\nwe never look at \"flags\" between the two lines. I think it would be more\nobvious to set them all in the same place.\n\n> +\t} else if (nul_term_line)\n> +\t\tdie(_(\"-z requires --stdin\"));\n> +\n\nHmm, there's our brace question coming up again. :)\n\n> diff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh\n> new file mode 100755\n> index 0000000000..997dc42dd2\n> --- /dev/null\n> +++ b/t/t7107-reset-stdin.sh\n> @@ -0,0 +1,33 @@\n> +#!/bin/sh\n> +\n> +test_description='reset --stdin'\n\nHere are a few things we might want to test that I didn't see covered:\n\n - feeding path X does not reset path Y\n\n - de-quoting in the non-z case\n\n-Peff\n"},{"id":"310415","messageId":"alpine.DEB.2.20.1701271832270.3469@virtualbox","threadId":"44275","inReplyTo":"20170127170422.lvpghp6jdud37zxx@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] reset: support the --stdin option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-27T17:34:46Z","receivedAt":"2017-01-27T17:35:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 27 Jan 2017, Jeff King wrote:\n\n> On Fri, Jan 27, 2017 at 01:38:55PM +0100, Johannes Schindelin wrote:\n> \n> A few minor suggestions:\n> \n> > +--stdin::\n> > +\tInstead of taking list of paths from the command line,\n> > +\tread list of paths from the standard input. Paths are\n> > +\tseparated by LF (i.e. one path per line) by default.\n> > +\n> > +-z::\n> > +\tOnly meaningful with `--stdin`; paths are separated with\n> > +\tNUL character instead of LF.\n> \n> Is it worth clarifying that these are paths, not pathspecs? The word\n> \"paths\" is used to refer to the pathspecs on the command-line elsewhere\n> in the document.\n> \n> It might also be worth mentioning the quoting rules for the non-z case.\n\nI think this would be overkill. In reality, --stdin without -z does not\nmake much sense, anyway.\n\nIf you feel strongly about it, I encourage you to submit a follow-up\npatch.\n\nThe rest of your suggestions have been implemented in v3.\n\nCiao,\nJohannes\n"},{"id":"310416","messageId":"c91ef43959ac94204fb37ab0060b8d2b516832e6.1485538197.git.johannes.schindelin@gmx.de","threadId":"44275","inReplyTo":"cover.1485538197.git.johannes.schindelin@gmx.de","subject":"[PATCH v3 1/1] reset: support the --stdin option","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2017-01-27T17:30:16Z","receivedAt":"2017-01-27T17:40:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Just like with other Git commands, this option makes it read the paths\nfrom the standard input. It comes in handy when resetting many, many\npaths at once and wildcards are not an option (e.g. when the paths are\ngenerated by a tool).\n\nNote: we first parse the entire list and perform the actual reset action\nonly in a second phase. Not only does this make things simpler, it also\nhelps performance, as do_diff_cache() traverses the index and the\n(sorted) pathspecs in simultaneously to avoid unnecessary lookups.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-reset.txt | 10 ++++++++++\n builtin/reset.c | 47 ++++++++++++++++++++++++++++++++++++++++++++-\n t/t7107-reset-stdin.sh | 33 +++++++++++++++++++++++++++++++\n 3 files changed, 89 insertions(+), 1 deletion(-)\n create mode 100755 t/t7107-reset-stdin.sh\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 25432d9257..d319ed9b20 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -10,6 +10,7 @@ SYNOPSIS\n [verse]\n 'git reset' [-q] [] [--] ...\n 'git reset' (--patch | -p) [] [--] [...]\n+'git reset' [-q] [--stdin [-z]] []\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n@@ -97,6 +98,15 @@ OPTIONS\n --quiet::\n \tBe quiet, only report errors.\n \n+--stdin::\n+\tInstead of taking list of paths from the command line,\n+\tread list of paths from the standard input. The paths are\n+\tread verbatim, i.e. not handled as pathspecs. Paths are\n+\tseparated by LF (i.e. one path per line) by default.\n+\n+-z::\n+\tOnly meaningful with `--stdin`; paths are separated with\n+\tNUL character instead of LF.\n \n EXAMPLES\n --------\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8ab915bfcb..fe7723c179 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -21,10 +21,14 @@\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n #include \"cache-tree.h\"\n+#include \"strbuf.h\"\n+#include \"quote.h\"\n+#include \"argv-array.h\"\n \n static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []\"),\n \tN_(\"git reset [-q] [] [--] ...\"),\n+\tN_(\"git reset [-q] [--stdin [-z]] []\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n@@ -267,7 +271,8 @@ static int reset_refs(const char *rev, const struct object_id *oid)\n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n-\tint patch_mode = 0, unborn;\n+\tint patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;\n+\tstruct argv_array stdin_paths = ARGV_ARRAY_INIT;\n \tconst char *rev;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n@@ -286,6 +291,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n \t\tOPT_BOOL('N', \"intent-to-add\", &intent_to_add,\n \t\t\t\tN_(\"record only the fact that removed paths will be added later\")),\n+\t\tOPT_BOOL('z', NULL, &nul_term_line,\n+\t\t\tN_(\"paths are separated with NUL character\")),\n+\t\tOPT_BOOL(0, \"stdin\", &read_from_stdin,\n+\t\t\t\tN_(\"read paths from \")),\n \t\tOPT_END()\n \t};\n \n@@ -295,6 +304,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \n+\tif (read_from_stdin) {\n+\t\tstrbuf_getline_fn getline_fn = nul_term_line ?\n+\t\t\tstrbuf_getline_nul : strbuf_getline_lf;\n+\t\tint flags = PATHSPEC_PREFER_FULL |\n+\t\t\tPATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP;\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tstruct strbuf unquoted = STRBUF_INIT;\n+\n+\t\tif (patch_mode)\n+\t\t\tdie(_(\"--stdin is incompatible with --patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--stdin is incompatible with path arguments\"));\n+\n+\t\twhile (getline_fn(&buf, stdin) != EOF) {\n+\t\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n+\t\t\t\tstrbuf_reset(&unquoted);\n+\t\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n+\t\t\t\t\tdie(_(\"line is badly quoted\"));\n+\t\t\t\tstrbuf_swap(&buf, &unquoted);\n+\t\t\t}\n+\t\t\targv_array_push(&stdin_paths, buf.buf);\n+\t\t\tstrbuf_reset(&buf);\n+\t\t}\n+\t\tstrbuf_release(&unquoted);\n+\t\tstrbuf_release(&buf);\n+\n+\t\tflags |= PATHSPEC_LITERAL_PATH;\n+\t\tparse_pathspec(&pathspec, 0, flags, prefix,\n+\t\t\t stdin_paths.argv);\n+\n+\t} else if (nul_term_line)\n+\t\tdie(_(\"-z requires --stdin\"));\n+\n \tunborn = !strcmp(rev, \"HEAD\") && get_sha1(\"HEAD\", oid.hash);\n \tif (unborn) {\n \t\t/* reset on unborn branch: treat as reset to empty tree */\n@@ -385,5 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (!pathspec.nr)\n \t\tremove_branch_state();\n \n+\targv_array_clear(&stdin_paths);\n+\n \treturn update_ref_status;\n }\ndiff --git a/t/t7107-reset-stdin.sh b/t/t7107-reset-stdin.sh\nnew file mode 100755\nindex 0000000000..997dc42dd2\n--- /dev/null\n+++ b/t/t7107-reset-stdin.sh\n@@ -0,0 +1,33 @@\n+#!/bin/sh\n+\n+test_description='reset --stdin'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'reset --stdin' '\n+\ttest_commit hello &&\n+\tgit rm hello.t &&\n+\ttest -z \"$(git ls-files hello.t)\" &&\n+\techo hello.t | git reset --stdin &&\n+\ttest hello.t = \"$(git ls-files hello.t)\"\n+'\n+\n+test_expect_success 'reset --stdin -z' '\n+\ttest_commit world &&\n+\tgit rm hello.t world.t &&\n+\ttest -z \"$(git ls-files hello.t world.t)\" &&\n+\tprintf world.tQworld.tQhello.tQ | q_to_nul | git reset --stdin -z &&\n+\tprintf \"hello.t\\nworld.t\\n\" >expect &&\n+\tgit ls-files >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--stdin requires --mixed' '\n+\techo hello.t >list &&\n+\ttest_must_fail git reset --soft --stdin ]\"),\n @@ -271,8 +272,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n \tint patch_mode = 0, nul_term_line = 0, read_from_stdin = 0, unborn;\n -\tchar **stdin_paths = NULL;\n -\tint stdin_nr = 0, stdin_alloc = 0;\n +\tstruct argv_array stdin_paths = ARGV_ARRAY_INIT;\n \tconst char *rev;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n @@ -325,18 +325,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tdie(_(\"line is badly quoted\"));\n \t\t\t\tstrbuf_swap(&buf, &unquoted);\n \t\t\t}\n -\t\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n -\t\t\tstdin_paths[stdin_nr++] = xstrdup(buf.buf);\n +\t\t\targv_array_push(&stdin_paths, buf.buf);\n \t\t\tstrbuf_reset(&buf);\n \t\t}\n \t\tstrbuf_release(&unquoted);\n \t\tstrbuf_release(&buf);\n \n -\t\tALLOC_GROW(stdin_paths, stdin_nr + 1, stdin_alloc);\n -\t\tstdin_paths[stdin_nr++] = NULL;\n \t\tflags |= PATHSPEC_LITERAL_PATH;\n \t\tparse_pathspec(&pathspec, 0, flags, prefix,\n -\t\t\t (const char **)stdin_paths);\n +\t\t\t stdin_paths.argv);\n \n \t} else if (nul_term_line)\n \t\tdie(_(\"-z requires --stdin\"));\n @@ -431,11 +428,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (!pathspec.nr)\n \t\tremove_branch_state();\n \n -\tif (stdin_paths) {\n -\t\twhile (stdin_nr)\n -\t\t\tfree(stdin_paths[--stdin_nr]);\n -\t\tfree(stdin_paths);\n -\t}\n +\targv_array_clear(&stdin_paths);\n \n \treturn update_ref_status;\n }\n\n-- \n2.11.1.windows.prerelease.2.9.g3014b57\n\n"},{"id":"310423","messageId":"20170127175422.b7bds4toeytndo43@sigill.intra.peff.net","threadId":"44275","inReplyTo":"alpine.DEB.2.20.1701271832270.3469@virtualbox","subject":"Re: [PATCH v2 1/1] reset: support the --stdin option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-27T17:54:23Z","receivedAt":"2017-01-27T17:58:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2017 at 06:34:46PM +0100, Johannes Schindelin wrote:\n\n> > Is it worth clarifying that these are paths, not pathspecs? The word\n> > \"paths\" is used to refer to the pathspecs on the command-line elsewhere\n> > in the document.\n> > \n> > It might also be worth mentioning the quoting rules for the non-z case.\n> \n> I think this would be overkill. In reality, --stdin without -z does not\n> make much sense, anyway.\n\nI think with most Unix tools people tend to use the non-z forms until\nthey break, and then switch to the z form. :)\n\nAnd in that sense, this transparently Just Works because the output will\noften come from another git tool, which will quote as appropriate.\n\n> If you feel strongly about it, I encourage you to submit a follow-up\n> patch.\n> \n> The rest of your suggestions have been implemented in v3.\n\nThanks. I think the path/pathspec thing was the more important of the\ntwo suggestions.\n\n-Peff\n"},{"id":"310429","messageId":"xmqqh94kz76v.fsf@gitster.mtv.corp.google.com","threadId":"44275","inReplyTo":"20170127170422.lvpghp6jdud37zxx@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] reset: support the --stdin option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-27T18:30:48Z","receivedAt":"2017-01-27T18:33:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> A few minor suggestions:\n>\n>> +--stdin::\n>> +\tInstead of taking list of paths from the command line,\n>> +\tread list of paths from the standard input. Paths are\n>> +\tseparated by LF (i.e. one path per line) by default.\n>> +\n>> +-z::\n>> +\tOnly meaningful with `--stdin`; paths are separated with\n>> +\tNUL character instead of LF.\n>\n> Is it worth clarifying that these are paths, not pathspecs? The word\n> \"paths\" is used to refer to the pathspecs on the command-line elsewhere\n> in the document.\n\nIf the code forces literal pathspecs, then what the user feeds to\nthe command is no longer pathspecs from the user's point of view,\nand I agree that the distinction is important. \n\nIf the remainder of the documentation is loose in terminology and\nthe reason why we were able to get away with it was because we\nconsistently used \"paths\" when we meant \"pathspec\", these existing\nmention of \"paths\" have to be tightened, either in this patch or a\npreparatory step patch before this one, because the addition of\n\"this thing reads paths not pathspecs\" is what makes them ambiguous.\n\nThanks. I re-read the part of the code that reads and unquotes as\nnecessary in the patch and they looked correct.\n\n\n\n\n"},{"id":"310449","messageId":"20170127221221.d53icsq7mdkbqm23@sigill.intra.peff.net","threadId":"44275","inReplyTo":"xmqqh94kz76v.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 1/1] reset: support the --stdin option","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-27T22:12:21Z","receivedAt":"2017-01-27T22:19:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 27, 2017 at 10:30:48AM -0800, Junio C Hamano wrote:\n\n> > Is it worth clarifying that these are paths, not pathspecs? The word\n> > \"paths\" is used to refer to the pathspecs on the command-line elsewhere\n> > in the document.\n> \n> If the code forces literal pathspecs, then what the user feeds to\n> the command is no longer pathspecs from the user's point of view,\n> and I agree that the distinction is important. \n> \n> If the remainder of the documentation is loose in terminology and\n> the reason why we were able to get away with it was because we\n> consistently used \"paths\" when we meant \"pathspec\", these existing\n> mention of \"paths\" have to be tightened, either in this patch or a\n> preparatory step patch before this one, because the addition of\n> \"this thing reads paths not pathspecs\" is what makes them ambiguous.\n\nI think a lot of the documentation uses to refer to pathspecs\n(e.g., git-log(1), git-diff(1), etc). As long as we're consistent with\nthat convention, I don't think it's that big a deal.\n\nThis spot needs a specific mention because it violates the convention.\n\nI don't know if the are other spots where it might be unclear, but I\nthink we're probably better to tighten those as they come up, rather\nthan switch to saying \"\" everywhere.\n\nThat's outside the scope of this series, though, I would think.\n\n-Peff\n"},{"id":"310455","messageId":"xmqqa8acxcfj.fsf@gitster.mtv.corp.google.com","threadId":"44275","inReplyTo":"20170127221221.d53icsq7mdkbqm23@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] reset: support the --stdin option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-28T00:20:32Z","receivedAt":"2017-01-28T00:20:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think a lot of the documentation uses to refer to pathspecs\n> (e.g., git-log(1), git-diff(1), etc). As long as we're consistent with\n> that convention, I don't think it's that big a deal.\n>\n> This spot needs a specific mention because it violates the convention.\n\nYup, I think we are in agreement.\n\n> I don't know if the are other spots where it might be unclear, but I\n> think we're probably better to tighten those as they come up, rather\n> than switch to saying \"\" everywhere.\n\nOh, I do not think I would disagree. As I think this change is an\ninstancethat makes the resulting text unclear, it would set a good\nexample to tighten existing one as part of its clean-up.\n\nIt can be done as a follow-up bugfix patch (i.e. \"previous one made\nthe resulting document uncleasr and here is to fix it\"), but that\nwould not serve as good ra ole model to mentor newer contributor as\ndoing the other way around.\n\n\n"}]}