{"thread":{"id":"51803","subject":"[PATCH 0/1] Teach git reset to optionally read the pathspecs from stdin","startedAt":"2019-09-04T21:38:33Z","lastAt":"2019-09-05T17:01:25Z","messageCount":6,"participants":["Johannes Schindelin via GitGitGadget","Eric Sunshine","Junio C Hamano","Alexandr Miloslavskiy"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"381833","messageId":"pull.133.git.gitgitgadget@gmail.com","threadId":"51803","inReplyTo":null,"subject":"[PATCH 0/1] Teach git reset to optionally read the pathspecs from stdin","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-04T21:38:29Z","receivedAt":"2019-09-04T21:38:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Especially in 3rd-party tools, where the shape of the repository/worktree is\nunpredictable, it is a wise thing to have an option to pass pathspec\nparameters via stdin rather than via the command line, as the latter has\nsize restrictions that the former does not have.\n\nJohannes Schindelin (1):\n reset: support the --stdin option\n\n Documentation/git-reset.txt | 11 +++++++++\n builtin/reset.c | 45 ++++++++++++++++++++++++++++++++++++-\n t/t7107-reset-stdin.sh | 41 +++++++++++++++++++++++++++++++++\n 3 files changed, 96 insertions(+), 1 deletion(-)\n create mode 100755 t/t7107-reset-stdin.sh\n\n\nbase-commit: 8104ec994ea3849a968b4667d072fedd1e688642\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-133%2Fdscho%2Freset-stdin-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-133/dscho/reset-stdin-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/133\n-- \ngitgitgadget\n"},{"id":"381834","messageId":"017945a55a64ebe1913d64919b699adb3f9d5d96.1567633109.git.gitgitgadget@gmail.com","threadId":"51803","inReplyTo":"pull.133.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] reset: support the --stdin option","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-04T21:38:30Z","receivedAt":"2019-09-04T21:38:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nJust 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. This make the implementation simpler than adding\nthe pathspecs immediately after the corresponding lines are read.\n\nNote also that we specifically handle CR/LF line endings in `stdin` to\nsupport Windows better.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-reset.txt | 11 +++++++++\n builtin/reset.c | 45 ++++++++++++++++++++++++++++++++++++-\n t/t7107-reset-stdin.sh | 41 +++++++++++++++++++++++++++++++++\n 3 files changed, 96 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 132f8e55f6..e03014eb04 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@@ -100,6 +101,16 @@ OPTIONS\n \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n \toverride the default behavior.\n \n+--stdin::\n+\tInstead of taking the list of paths from the command line,\n+\tread the list of paths from the standard input. The paths are\n+\tread verbatim, i.e. without special pathspecs handling. Paths\n+\tare expected to be separated by LF or CR/LF, unless the `-z`\n+\toption is in effect.\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 4d18a461fa..f8d06223d6 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -25,12 +25,16 @@\n #include \"cache-tree.h\"\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n+#include \"strbuf.h\"\n+#include \"quote.h\"\n+#include \"argv-array.h\"\n \n #define REFRESH_INDEX_DELAY_WARNING_IN_MS (2 * 1000)\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@@ -284,7 +288,8 @@ static int git_reset_config(const char *var, const char *value, void *cb)\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@@ -306,6 +311,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\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@@ -316,6 +325,38 @@ 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;\n+\t\tint flags = PATHSPEC_PREFER_FULL;\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, stdin_paths.argv);\n+\n+\t} else if (nul_term_line)\n+\t\tdie(_(\"-z requires --stdin\"));\n+\n \tunborn = !strcmp(rev, \"HEAD\") && get_oid(\"HEAD\", &oid);\n \tif (unborn) {\n \t\t/* reset on unborn branch: treat as reset to empty tree */\n@@ -416,5 +457,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tif (!pathspec.nr)\n \t\tremove_branch_state(the_repository);\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..db5483b8f1\n--- /dev/null\n+++ b/t/t7107-reset-stdin.sh\n@@ -0,0 +1,41 @@\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 list &&\n+\tgit reset --stdin wrote:\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> Signed-off-by: Johannes Schindelin \n> ---\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> @@ -316,6 +325,38 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n> + if (read_from_stdin) {\n> + [...]\n> + while (getline_fn(&buf, stdin) != EOF) {\n> + if (!nul_term_line && buf.buf[0] == '\"') {\n> + strbuf_reset(&unquoted);\n> + if (unquote_c_style(&unquoted, buf.buf, NULL))\n> + die(_(\"line is badly quoted\"));\n\nPerhaps include the offending line in the error message to make it\neasier for the user to understand what went wrong:\n\n die(_(\"line is badly quoted: %s\"), buf.buf);\n"},{"id":"381849","messageId":"xmqqblvzsl2i.fsf@gitster-ct.c.googlers.com","threadId":"51803","inReplyTo":"CAPig+cSw=qt6-b6xoyTGmtKRUdZnb=khy4eNV0_LSSNGu7x2tg@mail.gmail.com","subject":"Re: [PATCH 1/1] reset: support the --stdin option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-04T23:23:01Z","receivedAt":"2019-09-04T23:23:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> On Wed, Sep 4, 2019 at 5:38 PM Johannes Schindelin via GitGitGadget\n> wrote:\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>> Signed-off-by: Johannes Schindelin \n>> ---\n>> diff --git a/builtin/reset.c b/builtin/reset.c\n>> @@ -316,6 +325,38 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n>> + if (read_from_stdin) {\n>> + [...]\n>> + while (getline_fn(&buf, stdin) != EOF) {\n>> + if (!nul_term_line && buf.buf[0] == '\"') {\n>> + strbuf_reset(&unquoted);\n>> + if (unquote_c_style(&unquoted, buf.buf, NULL))\n>> + die(_(\"line is badly quoted\"));\n>\n> Perhaps include the offending line in the error message to make it\n> easier for the user to understand what went wrong:\n>\n> die(_(\"line is badly quoted: %s\"), buf.buf);\n\nI think the patch is solving the right problem by adding a feature\nthat is useful to work around the command line limit, but I doubt\nthe wisdom of limiting the input to \"paths\" by default. \n\nIt would be perfect if the command gets taught to take pathspec,\nwith \"git --[no-]literal-pathspec reset --stdin\" as an escape hatch.\n\n"},{"id":"381880","messageId":"fd300972-4fe7-54e4-3701-061ab4769c10@syntevo.com","threadId":"51803","inReplyTo":"017945a55a64ebe1913d64919b699adb3f9d5d96.1567633109.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] reset: support the --stdin option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-09-05T10:58:06Z","receivedAt":"2019-09-05T16:03:33Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Johannes, thanks for working on this problem!\n\nIn the previous discussion, there was a suggestion to change\n'--stdin' to '--paths-file', where '--paths-file -' would mean stdin:\nhttps://public-inbox.org/git/066cfd61-9700-e154-042f-fc9cffbd6346@web.de/\n\nI believe that was a good suggestion for a few reasons:\n1) Supporting files in addition to stdin sounds reasonable for its cost.\n2) '--paths-file' will support files and stdin with the same \"interface\",\n avoiding the possible need for another interface later.\n3) '--paths-file' sounds more self-documented then '--stdin'.\n\nLater, we intend to provide patches to extend the same feature to\nmultiple other commands, at least {add, checkout, commit, rm, stash},\nand I'm merely trying to avoid possible design issues for this\nlarger-scale change.\n\nIf you don't mind the suggestion but not willing to spend time\nimplementing it, we'd like to step in and contribute the remaining\nwork.\n\n"},{"id":"381881","messageId":"xmqqtv9qr82q.fsf@gitster-ct.c.googlers.com","threadId":"51803","inReplyTo":"fd300972-4fe7-54e4-3701-061ab4769c10@syntevo.com","subject":"Re: [PATCH 1/1] reset: support the --stdin option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-05T17:01:17Z","receivedAt":"2019-09-05T17:01:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexandr Miloslavskiy writes:\n\n> Johannes, thanks for working on this problem!\n>\n> In the previous discussion, there was a suggestion to change\n> '--stdin' to '--paths-file', where '--paths-file -' would mean stdin:\n> https://public-inbox.org/git/066cfd61-9700-e154-042f-fc9cffbd6346@web.de/\n>\n> I believe that was a good suggestion for a few reasons:\n> 1) Supporting files in addition to stdin sounds reasonable for its cost.\n> 2) '--paths-file' will support files and stdin with the same \"interface\",\n> avoiding the possible need for another interface later.\n> 3) '--paths-file' sounds more self-documented then '--stdin'.\n>\n> Later, we intend to provide patches to extend the same feature to\n> multiple other commands, at least {add, checkout, commit, rm, stash},\n> and I'm merely trying to avoid possible design issues for this\n> larger-scale change.\n>\n> If you don't mind the suggestion but not willing to spend time\n> implementing it, we'd like to step in and contribute the remaining\n> work.\n\nAhh, I completely forgot about the earlier exchange. Thanks for a\npointer.\n\nWhile I do like the idea of adding an option to take the pathspec,\nwhich you would usually give from the command line, from the\nstandard input, as a standard technique to lift the command line\nlength limit on various platforms, what I do not find right in the\nposted patch is that it conflates it with something orthogonal.\n\nWhen you already have a set of paths you would want to give to the\ncommand, and when these paths may have wildcard letters in them, you\nwould want to tell the command that you do not want wildcards in the\npathspec to take effect. That is a valid wish and we already have a\nstandard solution for that (i.e. the \"--literal-pathspecs\" option).\n\nBut it is orthogonal to how you feed these paths to the command. I\nwould not say \"unrelated\", in that there may be correlation between\nthe cases where you would want to feed pathspec from the standard\ninput and the cases where you would want these pathspec taken\nlitearlly, but the choice is still \"orthogonal\". \n\nIf we said \"when feeding from the standard input, they are taken\nliterally by default, but on the command line, the wildcards are\nexpanded\", that is bad enough---I do not think we want to hear any\nunnecessary \"Git UI is inconsistent\" noise raised from such a design\nchoice.\n\nThe posted patch does not even do that. It takes its input as\nliteral pathspec and it is done unconditionally without any way to\noverride it.\n\nIf we introduce a new --paths-from-file and make it take only paths\nand never pathspecs (i.e. no wildcard expansion, a directory does\nnot mean everything underneath it), that would be less worrisome wrt\nthe UI inconsistency front (but we may need to commit to teach\nexisting commands that take pathspec from the standard input the new\noption, too, so that all subcommands that can take some form of\npaths specification would work the same way with --paths-from-file).\n\nOr we can do --stdin (or --pathspec-from-file=-) that takes pathspecs,\nwhose literal-ness is controlled by the '--literal-pathspecs' option.\n\nThanks.\n\n"}]}