{"thread":{"id":"52185","subject":"[PATCH 0/5] Add --pathspec-from-file option for reset, commit","startedAt":"2019-11-04T19:26:50Z","lastAt":"2019-12-12T14:56:48Z","messageCount":80,"participants":["Alexandr Miloslavskiy via GitGitGadget","Phillip Wood","Alexandr Miloslavskiy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"385409","messageId":"pull.445.git.1572895605.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":null,"subject":"[PATCH 0/5] Add --pathspec-from-file option for reset, commit","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-04T19:26:40Z","receivedAt":"2019-11-04T19:26:50Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Adds --pathspec-from-file option for porcelain commands to avoid commandline\nlength limit.\n\nSo far I implemented it for git commit and git reset, but my goal is to\nsupport other commands as well after these patches are reviewed.\n\nThe patches are based on the following discussions:\nhttps://public-inbox.org/git/c3be6eff-365b-96b8-16d2-0528612fc1fc@syntevo.com/T/#u\nThere, --stdin-paths was suggested.\nhttps://public-inbox.org/git/a38bc928-7ccd-e2d9-b89b-23298e9fa95d@syntevo.com/T/#u\nThere, --stdin-paths was extended to --paths-file.\nhttps://public-inbox.org/git/pull.133.git.gitgitgadget@gmail.com/\nhttps://github.com/gitgitgadget/git/pull/133Patch from Johannes, I used it\nas base of my patch. There, --pathspec-from-file is suggested in discussion.\n\nMajor changes compared to patch from Johannes:\n\n1) --pathspec-from-file allows to read file, not just stdin 2) \n--literal-pathspecs should be honored. This is a good improvement because it\nkeeps --pathspec-from-file much closer to passing args on commandline. Since\nthe goal of the new option is to avoid commandline length limit, both\nbehaviors should be close to each other. 3) Patches are designed with all\nother git commands in mind\n\nAlexandr Miloslavskiy (5):\n pathspec: add new function to parse file\n doc: reset: unify description\n reset: support the --pathspec-from-file option\n doc: commit: unify description\n commit: support the --pathspec-from-file option\n\n Documentation/git-commit.txt | 19 ++++-\n Documentation/git-reset.txt | 40 +++++++---\n builtin/commit.c | 25 ++++++-\n builtin/reset.c | 22 +++++-\n pathspec.c | 41 +++++++++++\n pathspec.h | 10 +++\n t/t7107-reset-pathspec-file.sh | 126 ++++++++++++++++++++++++++++++++\n t/t7526-commit-pathspec-file.sh | 107 +++++++++++++++++++++++++++\n 8 files changed, 371 insertions(+), 19 deletions(-)\n create mode 100755 t/t7107-reset-pathspec-file.sh\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/445\n-- \ngitgitgadget\n"},{"id":"385410","messageId":"52e7a84a2ee61d6481286f6a32751107efc234d0.1572895605.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.git.1572895605.gitgitgadget@gmail.com","subject":"[PATCH 1/5] pathspec: add new function to parse file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-04T19:26:41Z","receivedAt":"2019-11-04T19:26:50Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis will be used to support the new option '--pathspec-from-file' in\n`git add`, `git-commit`, `git reset` etc.\n\nNote also that we specifically handle CR/LF line endings to support\nWindows better.\n\nTo simplify code, file is first parsed into `argv_array`. This allows\nto avoid refactoring `parse_pathspec()`.\n\nI considered adding `nul_term_line` to `flags` instead, but decided\nthat it doesn't fit there.\n\nThe new code is mostly taken from `cmd_update_index()` and\n`split_mail_conv()`.\n\nCo-authored-by: Johannes Schindelin \nSigned-off-by: Alexandr Miloslavskiy \n---\n pathspec.c | 41 +++++++++++++++++++++++++++++++++++++++++\n pathspec.h | 10 ++++++++++\n 2 files changed, 51 insertions(+)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 12c2b322b3..97d4e77875 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -3,6 +3,8 @@\n #include \"dir.h\"\n #include \"pathspec.h\"\n #include \"attr.h\"\n+#include \"argv-array.h\"\n+#include \"quote.h\"\n \n /*\n * Finds which of the given pathspecs match items in the index.\n@@ -613,6 +615,45 @@ void parse_pathspec(struct pathspec *pathspec,\n \t}\n }\n \n+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,\n+\t\t\t unsigned flags, const char *prefix,\n+\t\t\t const char *file, int nul_term_line)\n+{\n+\tstruct argv_array parsed_file = ARGV_ARRAY_INIT;\n+\tstrbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :\n+\t\t\t\t\t\t strbuf_getline;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf unquoted = STRBUF_INIT;\n+\tFILE *in = NULL;\n+\n+\tif (!strcmp(file, \"-\"))\n+\t\tin = stdin;\n+\telse\n+\t\tin = fopen(file, \"r\");\n+\n+\tif (!in)\n+\t\tdie(_(\"could not open '%s' for reading\"), file);\n+\n+\twhile (getline_fn(&buf, in) != EOF) {\n+\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&unquoted);\n+\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n+\t\t\t\tdie(_(\"line is badly quoted\"));\n+\t\t\tstrbuf_swap(&buf, &unquoted);\n+\t\t}\n+\t\targv_array_push(&parsed_file, buf.buf);\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\n+\tstrbuf_release(&unquoted);\n+\tstrbuf_release(&buf);\n+\tif (in != stdin)\n+\t\tfclose(in);\n+\n+\tparse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);\n+\targv_array_clear(&parsed_file);\n+}\n+\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src)\n {\n \tint i, j;\ndiff --git a/pathspec.h b/pathspec.h\nindex 1c18a2c90c..d72e0b4c4a 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,\n \t\t unsigned flags,\n \t\t const char *prefix,\n \t\t const char **args);\n+/*\n+ * Same as parse_pathspec() but uses file as input.\n+ * When 'file' is exactly \"-\" it uses 'stdin' instead.\n+ */\n+void parse_pathspec_file(struct pathspec *pathspec,\n+\t\t unsigned magic_mask,\n+\t\t unsigned flags,\n+\t\t const char *prefix,\n+\t\t const char *file,\n+\t\t int nul_term_line);\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src);\n void clear_pathspec(struct pathspec *);\n \n-- \ngitgitgadget\n\n"},{"id":"385412","messageId":"8d9f1fbc18346144a0c866a59891b652dcfe9b7f.1572895605.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.git.1572895605.gitgitgadget@gmail.com","subject":"[PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-04T19:26:43Z","receivedAt":"2019-11-04T19:26:53Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis option solves the problem of commandline length limit for UI's\nbuilt on top of git. Plumbing commands are not always a good fit, for\ntwo major reasons:\n1) Some UI's serve as assistants that help user run git commands. In\n this case, replacing familiar commands with plumbing commands will\n confuse most users.\n2) Some UI's have started and grown with porcelain commands. Replacing\n existing logic with plumbing commands could be cumbersome and prone\n to various new problems.\n\nThe new option is designed to behave very close to pathspecs passed in\ncommandline args, so that switching from one to another is simple.\n\nThe new option allows to read either a specified file or `stdin`.\nReading from file is a good way to avoid competing for stdin, and\nalso gives some extra flexibility.\n\nDecisions taken for simplicity:\n1) The new option is declared incompatible with other options that\n could use `stdin`.\n2) It is not allowed to pass some refspecs in args and others in file.\n3) New options do not have shorthands to avoid shorthand conflicts.\n\nAlso add new '--pathspec-file-null' switch that mirrors '-z' used in\nvarious places. Some porcelain commands, such as `git commit`, already\nuse '-z', therefore it needed a new unambiguous name.\n\nCo-authored-by: Johannes Schindelin \nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-reset.txt | 18 ++++-\n builtin/reset.c | 22 +++++-\n t/t7107-reset-pathspec-file.sh | 126 +++++++++++++++++++++++++++++++++\n 3 files changed, 161 insertions(+), 5 deletions(-)\n create mode 100755 t/t7107-reset-pathspec-file.sh\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex b0ea6e0ce5..d484cd2827 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -9,18 +9,20 @@ SYNOPSIS\n --------\n [verse]\n 'git reset' [-q] [] [--] ...\n+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] []\n 'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n -----------\n-In the first and second form, copy entries from `` to the index.\n-In the third form, set the current branch head (`HEAD`) to ``,\n+In the first three forms, copy entries from `` to the index.\n+In the last form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n \n 'git reset' [-q] [] [--] ...::\n-\tThis form resets the index entries for all `` to their\n+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] []::\n+\tThese forms reset the index entries for all `` to their\n \tstate at ``. (It does not affect the working tree or\n \tthe current branch.)\n +\n@@ -107,6 +109,16 @@ OPTIONS\n \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n \toverride the default behavior.\n \n+--pathspec-from-file=::\n+\tRead `` from `` instead. If `` is exactly `-`\n+\tthen read from standard input. Pathspecs are separated by LF or\n+\tCR/LF. Pathspecs can be quoted as explained for the configuration\n+\tvariable `core.quotePath` (see linkgit:git-config[1]). See also\n+\t`--pathspec-file-null` and global `--literal-pathspecs`.\n+\n+--pathspec-file-null::\n+\tOnly meaningful with `--pathspec-from-file`. Pathspecs are\n+\tseparated with NUL character and are not expected to be quoted.\n \n EXAMPLES\n --------\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fdd572168b..0eaa6b0bca 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -31,6 +31,7 @@\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] [--pathspec-from-file [--pathspec-file-null]] []\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n@@ -284,8 +285,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-\tconst char *rev;\n+\tint patch_mode = 0, pathspec_file_null = 0, unborn;\n+\tconst char *rev, *pathspec_from_file = NULL;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n \tint intent_to_add = 0;\n@@ -306,6 +307,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_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file,\n+\t\t\t\tN_(\"read pathspecs from file\")),\n+\t\tOPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null,\n+\t\t\t\tN_(\"with --pathspec-from-file, pathspecs are separated with NUL character\")),\n \t\tOPT_END()\n \t};\n \n@@ -316,6 +321,19 @@ 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 (pathspec_from_file) {\n+\t\tif (patch_mode)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with --patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n+\n+\t\tparse_pathspec_file(&pathspec, 0,\n+\t\t\t\t PATHSPEC_PREFER_FULL,\n+\t\t\t\t prefix, pathspec_from_file, pathspec_file_null);\n+\t} else if (pathspec_file_null)\n+\t\tdie(_(\"--pathspec-file-null requires --pathspec-from-file\"));\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 */\ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nnew file mode 100755\nindex 0000000000..cf7f085ad5\n--- /dev/null\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -0,0 +1,126 @@\n+#!/bin/sh\n+\n+test_description='reset --pathspec-from-file'\n+\n+. ./test-lib.sh\n+\n+cat > expect.a < expect.ab < expect.a_bc_d <fileA.t &&\n+\techo B >fileB.t &&\n+\techo C >fileC.t &&\n+\techo D >fileD.t &&\n+\tgit add . &&\n+\tgit commit --include . -m \"Commit\" &&\n+\tcheckpoint=$(git rev-parse --verify HEAD)\n+'\n+\n+restore_checkpoint () {\n+\tgit reset --hard \"$checkpoint\"\n+}\n+\n+verify_state () {\n+\tgit status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&\n+\ttest_cmp \"$1\" actual\n+}\n+\n+test_expect_success '--pathspec-from-file from stdin' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t | git reset --pathspec-from-file=- &&\n+\n+\tverify_state expect.a\n+'\n+\n+test_expect_success '--pathspec-from-file from file' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t >list &&\n+\tgit reset --pathspec-from-file=list &&\n+\n+\tverify_state expect.a\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf fileA.tQfileB.t | q_to_nul | git reset --pathspec-from-file=- --pathspec-file-null &&\n+\n+\tverify_state expect.ab\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n+\n+\tverify_state expect.ab\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\r\\nfileB.t\" | git reset --pathspec-from-file=- &&\n+\n+\tverify_state expect.ab\n+'\n+\n+test_expect_success 'quotes' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n+\n+\tverify_state expect.a\n+'\n+\n+test_expect_success 'quotes not compatible with --pathspec-file-null' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n+\tgit reset --pathspec-from-file=list --pathspec-file-null &&\n+\t\n+\ttest_must_fail verify_state expect.a\n+'\n+\n+test_expect_success '--pathspec-from-file is not compatible with --soft --hard' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t >list &&\n+\ttest_must_fail git reset --soft --pathspec-from-file=list &&\n+\ttest_must_fail git reset --hard --pathspec-from-file=list\n+'\n+\n+test_expect_success 'only touches what was listed' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\" | git reset --pathspec-from-file=- &&\n+\n+\tverify_state expect.a_bc_d\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"385411","messageId":"251d06e27f6bc93b190450ae6e1087a3126b5e52.1572895605.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.git.1572895605.gitgitgadget@gmail.com","subject":"[PATCH 4/5] doc: commit: unify description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-04T19:26:44Z","receivedAt":"2019-11-04T19:26:54Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSynchronize it to `git add`, which has a pretty good description.\nThis also better disambiguates ... header.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex afa7b75a23..4341d0e3ab 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -345,12 +345,15 @@ changes to tracked files.\n \\--::\n \tDo not interpret any more arguments as options.\n \n-...::\n+...::\n \tWhen files are given on the command line, the command\n \tcommits the contents of the named files, without\n \trecording the changes already staged. The contents of\n \tthese files are also staged for the next commit on top\n \tof what have been staged before.\n++\n+For more details about the syntax, see the 'pathspec' entry\n+in linkgit:gitglossary[7].\n \n :git-commit: 1\n include::date-formats.txt[]\n-- \ngitgitgadget\n\n"},{"id":"385413","messageId":"f4847046896848d3f16bc5f3cb7a26271cefd97c.1572895605.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.git.1572895605.gitgitgadget@gmail.com","subject":"[PATCH 5/5] commit: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-04T19:26:45Z","receivedAt":"2019-11-04T19:26:55Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis option solves the problem of commandline length limit for UI's\nbuilt on top of git. Plumbing commands are not always a good fit, for\ntwo major reasons:\n1) Some UI's serve as assistants that help user run git commands. In\n this case, replacing familiar commands with plumbing commands will\n confuse most users.\n2) Some UI's have started and grown with porcelain commands. Replacing\n existing logic with plumbing commands could be cumbersome and prone\n to various new problems.\n\nThe new option is designed to behave very close to pathspecs passed in\ncommandline args, so that switching from one to another is simple.\n\nThe new option allows to read either a specified file or `stdin`.\nReading from file is a good way to avoid competing for stdin, and\nalso gives some extra flexibility.\n\nDecisions taken for simplicity:\n1) The new option is declared incompatible with other options that\n could use `stdin`.\n2) It is not allowed to pass some refspecs in args and others in file.\n3) New options do not have shorthands to avoid shorthand conflicts.\n\nAlso add new '--pathspec-file-null' switch that mirrors '-z' used in\nvarious places. Some porcelain commands, such as `git commit`, already\nuse '-z', therefore it needed a new unambiguous name.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 14 ++++-\n builtin/commit.c | 25 ++++++--\n t/t7526-commit-pathspec-file.sh | 107 ++++++++++++++++++++++++++++++++\n 3 files changed, 141 insertions(+), 5 deletions(-)\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 4341d0e3ab..ec4752298d 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,8 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [--pathspec-from-file= [--pathspec-file-null]]\n+\t [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -277,6 +278,17 @@ FROM UPSTREAM REBASE\" section in linkgit:git-rebase[1].)\n \tthe last commit without committing changes that have\n \talready been staged. If used together with `--allow-empty`\n \tpaths are also not required, and an empty commit will be created.\n+\t\n+--pathspec-from-file=::\n+\tRead `` from `` instead. If `` is exactly `-`\n+\tthen read from standard input. Pathspecs are separated by LF or\n+\tCR/LF. Pathspecs can be quoted as explained for the configuration\n+\tvariable `core.quotePath` (see linkgit:git-config[1]). See also\n+\t`--pathspec-file-null` and global `--literal-pathspecs`.\n+\n+--pathspec-file-null::\n+\tOnly meaningful with `--pathspec-from-file`. Pathspecs are\n+\tseparated with NUL character and are not expected to be quoted.\n \n -u[]::\n --untracked-files[=]::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e588bc6ad3..532f305926 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n-static int no_post_rewrite, allow_empty_message;\n+static int no_post_rewrite, allow_empty_message, pathspec_file_null;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n-static char *sign_commit;\n+static char *sign_commit, *pathspec_from_file;\n \n /*\n * The default commit message cleanup mode will remove the lines\n@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\t PATHSPEC_PREFER_FULL,\n \t\t prefix, argv);\n \n+\tif (pathspec_from_file) {\n+\t\tif (interactive)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n+\n+\t\tparse_pathspec_file(&pathspec, 0,\n+\t\t\t\t PATHSPEC_PREFER_FULL,\n+\t\t\t\t prefix, pathspec_from_file, pathspec_file_null);\n+\t}\n+\telse if (pathspec_file_null)\n+\t\tdie(_(\"--pathspec-file-null requires --pathspec-from-file\"));\n+\n+\tif (!pathspec.nr && (also || (only && !amend && !allow_empty)))\n+\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n+\n \tif (read_cache_preload(&pathspec) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (also + only + all + interactive > 1)\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n-\tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n-\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n \tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n \n \thandle_untracked_files_arg(s);\n@@ -1535,6 +1550,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"amend\", &amend, N_(\"amend previous commit\")),\n \t\tOPT_BOOL(0, \"no-post-rewrite\", &no_post_rewrite, N_(\"bypass post-rewrite hook\")),\n \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n+\t\tOPT_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file, N_(\"read pathspecs from file\")),\n+\t\tOPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null, N_(\"with --pathspec-from-file, pathspecs are separated with NUL character\")),\n \t\t/* end commit contents options */\n \n \t\tOPT_HIDDEN_BOOL(0, \"allow-empty\", &allow_empty,\ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nnew file mode 100755\nindex 0000000000..c5d68e01e6\n--- /dev/null\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -0,0 +1,107 @@\n+#!/bin/sh\n+\n+test_description='commit --pathspec-from-file'\n+\n+. ./test-lib.sh\n+\n+test_tick\n+\n+cat > expect.a < expect.ab < expect.bc <fileA.t &&\n+\techo B >fileB.t &&\n+\techo C >fileC.t &&\n+\techo D >fileD.t &&\n+\tgit add fileA.t fileB.t fileC.t fileD.t\n+'\n+\n+restore_checkpoint () {\n+\tgit reset --soft \"$checkpoint\"\n+}\n+\n+verify_state () {\n+\tgit diff-tree --no-commit-id --name-status -r HEAD >actual &&\n+\ttest_cmp \"$1\" actual\n+}\n+\n+test_expect_success '--pathspec-from-file from stdin' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tverify_state expect.a\n+'\n+\n+test_expect_success '--pathspec-from-file from file' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t >list &&\n+\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n+\n+\tverify_state expect.a\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf fileA.tQfileB.t | q_to_nul | git commit --pathspec-from-file=- --pathspec-file-null -m \"Commit\" &&\n+\n+\tverify_state expect.ab\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tverify_state expect.ab\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\r\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tverify_state expect.ab\n+'\n+\n+test_expect_success 'quotes' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tverify_state expect.a\n+'\n+\n+test_expect_success 'quotes not compatible with --pathspec-file-null' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-null -m \"Commit\"\n+'\n+\n+test_expect_success 'only touches what was listed' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileB.t\\nfileC.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tverify_state expect.bc\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"385414","messageId":"1740ac7a291cfc81418c2d437201c3373487fa26.1572895605.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.git.1572895605.gitgitgadget@gmail.com","subject":"[PATCH 2/5] doc: reset: unify description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-04T19:26:42Z","receivedAt":"2019-11-04T19:26:58Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSynchronize it to `git add`, which has a pretty good description.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-reset.txt | 24 +++++++++++++++---------\n 1 file changed, 15 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 97e0544d9e..b0ea6e0ce5 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state\n SYNOPSIS\n --------\n [verse]\n-'git reset' [-q] [] [--] ...\n-'git reset' (--patch | -p) [] [--] [...]\n+'git reset' [-q] [] [--] ...\n+'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n@@ -19,27 +19,33 @@ In the third form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n \n-'git reset' [-q] [] [--] ...::\n-\tThis form resets the index entries for all `` to their\n+'git reset' [-q] [] [--] ...::\n+\tThis form resets the index entries for all `` to their\n \tstate at ``. (It does not affect the working tree or\n \tthe current branch.)\n +\n-This means that `git reset ` is the opposite of `git add\n-`. This command is equivalent to\n-`git restore [--source=] --staged ...`.\n+For more details about the syntax, see the 'pathspec' entry\n+in linkgit:gitglossary[7].\n +\n-After running `git reset ` to update the index entry, you can\n+This means that `git reset ` is the opposite of `git add\n+`. This command is equivalent to\n+`git restore [--source=] --staged ...`.\n++\n+After running `git reset ` to update the index entry, you can\n use linkgit:git-restore[1] to check the contents out of the index to\n the working tree. Alternatively, using linkgit:git-restore[1]\n and specifying a commit with `--source`, you\n can copy the contents of a path out of a commit to the index and to the\n working tree in one go.\n \n-'git reset' (--patch | -p) [] [--] [...]::\n+'git reset' (--patch | -p) [] [--] [...]::\n \tInteractively select hunks in the difference between the index\n \tand `` (defaults to `HEAD`). The chosen hunks are applied\n \tin reverse to the index.\n +\n+For more details about the syntax, see the 'pathspec' entry\n+in linkgit:gitglossary[7].\n++\n This means that `git reset -p` is the opposite of `git add -p`, i.e.\n you can use it to selectively reset hunks. See the ``Interactive Mode''\n section of linkgit:git-add[1] to learn how to operate the `--patch` mode.\n-- \ngitgitgadget\n\n"},{"id":"385449","messageId":"e34eba95-f413-ddd3-08c6-4f5b2bed3761@gmail.com","threadId":"52185","inReplyTo":"52e7a84a2ee61d6481286f6a32751107efc234d0.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/5] pathspec: add new function to parse file","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-11-05T15:02:18Z","receivedAt":"2019-11-05T15:02:26Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alexandr\n\nThanks for working on this. I've got a couple of comments about \nimproving the error messages but this looks fine to me\n\nOn 04/11/2019 19:26, Alexandr Miloslavskiy via GitGitGadget wrote:\n> From: Alexandr Miloslavskiy \n> \n> This will be used to support the new option '--pathspec-from-file' in\n> `git add`, `git-commit`, `git reset` etc.\n> \n> Note also that we specifically handle CR/LF line endings to support\n> Windows better.\n> \n> To simplify code, file is first parsed into `argv_array`. This allows\n> to avoid refactoring `parse_pathspec()`.\n> \n> I considered adding `nul_term_line` to `flags` instead, but decided\n> that it doesn't fit there.\n> \n> The new code is mostly taken from `cmd_update_index()` and\n> `split_mail_conv()`.\n> \n> Co-authored-by: Johannes Schindelin \n> Signed-off-by: Alexandr Miloslavskiy \n> ---\n> pathspec.c | 41 +++++++++++++++++++++++++++++++++++++++++\n> pathspec.h | 10 ++++++++++\n> 2 files changed, 51 insertions(+)\n> \n> diff --git a/pathspec.c b/pathspec.c\n> index 12c2b322b3..97d4e77875 100644\n> --- a/pathspec.c\n> +++ b/pathspec.c\n> @@ -3,6 +3,8 @@\n> #include \"dir.h\"\n> #include \"pathspec.h\"\n> #include \"attr.h\"\n> +#include \"argv-array.h\"\n> +#include \"quote.h\"\n> \n> /*\n> * Finds which of the given pathspecs match items in the index.\n> @@ -613,6 +615,45 @@ void parse_pathspec(struct pathspec *pathspec,\n> \t}\n> }\n> \n> +void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,\n> +\t\t\t unsigned flags, const char *prefix,\n> +\t\t\t const char *file, int nul_term_line)\n> +{\n> +\tstruct argv_array parsed_file = ARGV_ARRAY_INIT;\n> +\tstrbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :\n> +\t\t\t\t\t\t strbuf_getline;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tstruct strbuf unquoted = STRBUF_INIT;\n> +\tFILE *in = NULL;\n> +\n> +\tif (!strcmp(file, \"-\"))\n> +\t\tin = stdin;\n> +\telse\n> +\t\tin = fopen(file, \"r\");\n> +\n> +\tif (!in)\n> +\t\tdie(_(\"could not open '%s' for reading\"), file);\n\ndie_errno() would give a more informative message here\n\n> +\twhile (getline_fn(&buf, in) != EOF) {\n> +\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n> +\t\t\tstrbuf_reset(&unquoted);\n> +\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n> +\t\t\t\tdie(_(\"line is badly quoted\"));\n\nIt would be nice to show the offending line in the error message\n\n> +\t\t\tstrbuf_swap(&buf, &unquoted);\n> +\t\t}\n> +\t\targv_array_push(&parsed_file, buf.buf);\n> +\t\tstrbuf_reset(&buf);\n> +\t}\n> +\n> +\tstrbuf_release(&unquoted);\n> +\tstrbuf_release(&buf);\n> +\tif (in != stdin)\n> +\t\tfclose(in);\n> +\n> +\tparse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);\n> +\targv_array_clear(&parsed_file);\n> +}\n> +\n> void copy_pathspec(struct pathspec *dst, const struct pathspec *src)\n> {\n> \tint i, j;\n> diff --git a/pathspec.h b/pathspec.h\n> index 1c18a2c90c..d72e0b4c4a 100644\n> --- a/pathspec.h\n> +++ b/pathspec.h\n> @@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,\n> \t\t unsigned flags,\n> \t\t const char *prefix,\n> \t\t const char **args);\n> +/*\n> + * Same as parse_pathspec() but uses file as input.\n> + * When 'file' is exactly \"-\" it uses 'stdin' instead.\n> + */\n> +void parse_pathspec_file(struct pathspec *pathspec,\n> +\t\t unsigned magic_mask,\n> +\t\t unsigned flags,\n> +\t\t const char *prefix,\n> +\t\t const char *file,\n> +\t\t int nul_term_line);\n\nDo these align with the 's' in \"struct pathspec\" ?\n\nBest Wishes\n\nPhillip\n\n> void copy_pathspec(struct pathspec *dst, const struct pathspec *src);\n> void clear_pathspec(struct pathspec *);\n> \n> \n"},{"id":"385450","messageId":"b93592df-561f-36d4-09ce-e02d570c60da@gmail.com","threadId":"52185","inReplyTo":"8d9f1fbc18346144a0c866a59891b652dcfe9b7f.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-11-05T15:03:56Z","receivedAt":"2019-11-05T15:04:05Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alexandr\n\nThis also looks good, I've got some minor comments below. If I'm \ncomplaining about whitespace and style issues you know the patch is \nquite good!\n\nOn 04/11/2019 19:26, Alexandr Miloslavskiy via GitGitGadget wrote:\n> From: Alexandr Miloslavskiy \n> \n> This option solves the problem of commandline length limit for UI's\n> built on top of git. Plumbing commands are not always a good fit, for\n> two major reasons:\n> 1) Some UI's serve as assistants that help user run git commands. In\n> this case, replacing familiar commands with plumbing commands will\n> confuse most users.\n> 2) Some UI's have started and grown with porcelain commands. Replacing\n> existing logic with plumbing commands could be cumbersome and prone\n> to various new problems.\n\nI think reset is a good example of the second reason - it's not straight \nforward to implement it in plumbing commands\n\n> The new option is designed to behave very close to pathspecs passed in\n> commandline args, so that switching from one to another is simple.\n> \n> The new option allows to read either a specified file or `stdin`.\n> Reading from file is a good way to avoid competing for stdin, and\n> also gives some extra flexibility.\n\nI think this flexibility is a good idea\n\n> Decisions taken for simplicity:\n> 1) The new option is declared incompatible with other options that\n> could use `stdin`.\n\nI'm confused reset does not use stdin does it?\n\n> 2) It is not allowed to pass some refspecs in args and others in file.\n\ns/refspecs/pathspecs/ ?\n\n> 3) New options do not have shorthands to avoid shorthand conflicts.\n\n> Also add new '--pathspec-file-null' switch that mirrors '-z' used in\n> various places. Some porcelain commands, such as `git commit`, already\n> use '-z', therefore it needed a new unambiguous name.\n\nAs the 'lines' in the file are nul terminated perhaps it would be better \nto call this --pathspec-file-nul or --nul-termination. I think the use \nof --null to mean nul termination for config was a mistake (for grep it \nmatches what GUN grep does but it's still unfortunate).\n\n> \n> Co-authored-by: Johannes Schindelin \n> Signed-off-by: Alexandr Miloslavskiy \n> ---\n> Documentation/git-reset.txt | 18 ++++-\n> builtin/reset.c | 22 +++++-\n> t/t7107-reset-pathspec-file.sh | 126 +++++++++++++++++++++++++++++++++\n> 3 files changed, 161 insertions(+), 5 deletions(-)\n> create mode 100755 t/t7107-reset-pathspec-file.sh\n> \n> diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\n> index b0ea6e0ce5..d484cd2827 100644\n> --- a/Documentation/git-reset.txt\n> +++ b/Documentation/git-reset.txt\n> @@ -9,18 +9,20 @@ SYNOPSIS\n> --------\n> [verse]\n> 'git reset' [-q] [] [--] ...\n> +'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] []\n\n--pathspec-file would be shorter and still conveys the intent of the \noption. Is this line missing a leading space?\n\n> 'git reset' (--patch | -p) [] [--] [...]\n> 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n> \n> DESCRIPTION\n> -----------\n> -In the first and second form, copy entries from `` to the index.\n> -In the third form, set the current branch head (`HEAD`) to ``,\n> +In the first three forms, copy entries from `` to the index.\n> +In the last form, set the current branch head (`HEAD`) to ``,\n> optionally modifying index and working tree to match.\n> The ``/`` defaults to `HEAD` in all forms.\n> \n> 'git reset' [-q] [] [--] ...::\n> -\tThis form resets the index entries for all `` to their\n> +'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] []::\n\nAlignment again\n\n> +\tThese forms reset the index entries for all `` to their\n\nThe new form does not mention so this could be confusing\n\n> \tstate at ``. (It does not affect the working tree or\n> \tthe current branch.)\n> +\n> @@ -107,6 +109,16 @@ OPTIONS\n> \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n> \toverride the default behavior.\n> \n> +--pathspec-from-file=::\n> +\tRead `` from `` instead. \n\nAs we have a separate synopsis line for --pathspec-from-file which does \nnot mention it might be better just to say \"read pathspecs \nfrom `` instead of the command line\".\n\n> If `` is exactly `-`\n> +\tthen read from standard input. Pathspecs are separated by LF or\n> +\tCR/LF. Pathspecs can be quoted as explained for the configuration\n> +\tvariable `core.quotePath` (see linkgit:git-config[1]). See also\n> +\t`--pathspec-file-null` and global `--literal-pathspecs`.\n> +\n> +--pathspec-file-null::\n> +\tOnly meaningful with `--pathspec-from-file`. Pathspecs are\n> +\tseparated with NUL character and are not expected to be quoted.\n> \n> EXAMPLES\n> --------\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index fdd572168b..0eaa6b0bca 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -31,6 +31,7 @@\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] [--pathspec-from-file [--pathspec-file-null]] []\"),\n> \tN_(\"git reset --patch [] [--] [...]\"),\n> \tNULL\n> };\n> @@ -284,8 +285,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> -\tconst char *rev;\n> +\tint patch_mode = 0, pathspec_file_null = 0, unborn;\n> +\tconst char *rev, *pathspec_from_file = NULL;\n> \tstruct object_id oid;\n> \tstruct pathspec pathspec;\n> \tint intent_to_add = 0;\n> @@ -306,6 +307,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_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file,\n> +\t\t\t\tN_(\"read pathspecs from file\")),\n> +\t\tOPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null,\n> +\t\t\t\tN_(\"with --pathspec-from-file, pathspecs are separated with NUL character\")),\n> \t\tOPT_END()\n> \t};\n> \n> @@ -316,6 +321,19 @@ 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 (pathspec_from_file) {\n> +\t\tif (patch_mode)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with --patch\"));\n\nThis is sensible as -p is interactive so we wouldn't expect command line \nlength to be an issue\n\n> +\n> +\t\tif (pathspec.nr)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n> +\n> +\t\tparse_pathspec_file(&pathspec, 0,\n> +\t\t\t\t PATHSPEC_PREFER_FULL,\n> +\t\t\t\t prefix, pathspec_from_file, pathspec_file_null);\n> +\t} else if (pathspec_file_null)\n> +\t\tdie(_(\"--pathspec-file-null requires --pathspec-from-file\"));\n\nStyle nit: the coding guidelines state that if any branch of an if \nstatement requires braces then all the branches should be braced. This \nis widely ignored though.\n\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> diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\n> new file mode 100755\n> index 0000000000..cf7f085ad5\n> --- /dev/null\n> +++ b/t/t7107-reset-pathspec-file.sh\n> @@ -0,0 +1,126 @@\n> +#!/bin/sh\n> +\n> +test_description='reset --pathspec-from-file'\n> +\n> +. ./test-lib.sh\n> +\n> +cat > expect.a < + D fileA.t\n> +EOF\n> +\n> +cat > expect.ab < + D fileA.t\n> + D fileB.t\n> +EOF\n> +\n> +cat > expect.a_bc_d < +D fileA.t\n> + D fileB.t\n> + D fileC.t\n> +D fileD.t\n> +EOF\n\nThese days we tend to set up the expected files within the relevant test \ncase using <<-\\EOF to allow indentation and disallow substitution \n(unless it's needed of course)\n\n> +test_expect_success setup '\n> +\techo A >fileA.t &&\n> +\techo B >fileB.t &&\n> +\techo C >fileC.t &&\n> +\techo D >fileD.t &&\n> +\tgit add . &&\n> +\tgit commit --include . -m \"Commit\" &&\n> +\tcheckpoint=$(git rev-parse --verify HEAD)\n> +'\n> +\n> +restore_checkpoint () {\n> +\tgit reset --hard \"$checkpoint\"\n> +}\n> +\n> +verify_state () {\n> +\tgit status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&\n> +\ttest_cmp \"$1\" actual\n> +}\n> +\n> +test_expect_success '--pathspec-from-file from stdin' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success '--pathspec-from-file from file' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t >list &&\n> +\tgit reset --pathspec-from-file=list &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success 'NUL delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t &&\n> +\tprintf fileA.tQfileB.t | q_to_nul | git reset --pathspec-from-file=- --pathspec-file-null &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'LF delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t &&\n> +\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'CRLF delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t &&\n> +\tprintf \"fileA.t\\r\\nfileB.t\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'quotes' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.a\n\nIf I've understood correctly this doesn't test if a path is correctly \nunquoted, only that it is accepted.\n\n> +'\n> +\n> +test_expect_success 'quotes not compatible with --pathspec-file-null' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n> +\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n> +\tgit reset --pathspec-from-file=list --pathspec-file-null &&\n> +\t\n> +\ttest_must_fail verify_state expect.a\n> +'\n> +\n> +test_expect_success '--pathspec-from-file is not compatible with --soft --hard' '\n\ns/--soft --hard/--soft or --hard/\n\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t >list &&\n> +\ttest_must_fail git reset --soft --pathspec-from-file=list &&\n> +\ttest_must_fail git reset --hard --pathspec-from-file=list\n> +'\n> +\n> +test_expect_success 'only touches what was listed' '\n\ns/^/--pathspec-from-file / ?\n\nBest Wishes\n\nPhillip\n\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n> +\tprintf \"fileB.t\\nfileC.t\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.a_bc_d\n> +'\n> +\n> +test_done\n> \n"},{"id":"385459","messageId":"3cee48dd-15bb-1bec-6d23-d05cce0f4eb1@gmail.com","threadId":"52185","inReplyTo":"8d9f1fbc18346144a0c866a59891b652dcfe9b7f.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-11-05T16:14:08Z","receivedAt":"2019-11-05T16:14:15Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alexandr\n\nOn 04/11/2019 19:26, Alexandr Miloslavskiy via GitGitGadget wrote:\n> From: Alexandr Miloslavskiy \n> [...]\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index fdd572168b..0eaa6b0bca 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -31,6 +31,7 @@\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] [--pathspec-from-file [--pathspec-file-null]] []\"),\n> \tN_(\"git reset --patch [] [--] [...]\"),\n> \tNULL\n> };\n> @@ -284,8 +285,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> -\tconst char *rev;\n> +\tint patch_mode = 0, pathspec_file_null = 0, unborn;\n> +\tconst char *rev, *pathspec_from_file = NULL;\n> \tstruct object_id oid;\n> \tstruct pathspec pathspec;\n> \tint intent_to_add = 0;\n> @@ -306,6 +307,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_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file,\n> +\t\t\t\tN_(\"read pathspecs from file\")),\n> +\t\tOPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null,\n> +\t\t\t\tN_(\"with --pathspec-from-file, pathspecs are separated with NUL character\")),\n\nOne thing I forgot to mention before is that if you're going to add \nthese options to a number of commands then maybe it is worth defining a \nmacro for each one in parse-options.h. There are a couple of examples of \nthis at the end of that file.\n\nBest Wishes\n\nPhillip\n\n> \t\tOPT_END()\n> \t};\n> \n> @@ -316,6 +321,19 @@ 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 (pathspec_from_file) {\n> +\t\tif (patch_mode)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with --patch\"));\n> +\n> +\t\tif (pathspec.nr)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n> +\n> +\t\tparse_pathspec_file(&pathspec, 0,\n> +\t\t\t\t PATHSPEC_PREFER_FULL,\n> +\t\t\t\t prefix, pathspec_from_file, pathspec_file_null);\n> +\t} else if (pathspec_file_null)\n> +\t\tdie(_(\"--pathspec-file-null requires --pathspec-from-file\"));\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> diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\n> new file mode 100755\n> index 0000000000..cf7f085ad5\n> --- /dev/null\n> +++ b/t/t7107-reset-pathspec-file.sh\n> @@ -0,0 +1,126 @@\n> +#!/bin/sh\n> +\n> +test_description='reset --pathspec-from-file'\n> +\n> +. ./test-lib.sh\n> +\n> +cat > expect.a < + D fileA.t\n> +EOF\n> +\n> +cat > expect.ab < + D fileA.t\n> + D fileB.t\n> +EOF\n> +\n> +cat > expect.a_bc_d < +D fileA.t\n> + D fileB.t\n> + D fileC.t\n> +D fileD.t\n> +EOF\n> +\n> +test_expect_success setup '\n> +\techo A >fileA.t &&\n> +\techo B >fileB.t &&\n> +\techo C >fileC.t &&\n> +\techo D >fileD.t &&\n> +\tgit add . &&\n> +\tgit commit --include . -m \"Commit\" &&\n> +\tcheckpoint=$(git rev-parse --verify HEAD)\n> +'\n> +\n> +restore_checkpoint () {\n> +\tgit reset --hard \"$checkpoint\"\n> +}\n> +\n> +verify_state () {\n> +\tgit status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&\n> +\ttest_cmp \"$1\" actual\n> +}\n> +\n> +test_expect_success '--pathspec-from-file from stdin' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success '--pathspec-from-file from file' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t >list &&\n> +\tgit reset --pathspec-from-file=list &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success 'NUL delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t &&\n> +\tprintf fileA.tQfileB.t | q_to_nul | git reset --pathspec-from-file=- --pathspec-file-null &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'LF delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t &&\n> +\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'CRLF delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t &&\n> +\tprintf \"fileA.t\\r\\nfileB.t\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'quotes' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success 'quotes not compatible with --pathspec-file-null' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n> +\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n> +\tgit reset --pathspec-from-file=list --pathspec-file-null &&\n> +\t\n> +\ttest_must_fail verify_state expect.a\n> +'\n> +\n> +test_expect_success '--pathspec-from-file is not compatible with --soft --hard' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t >list &&\n> +\ttest_must_fail git reset --soft --pathspec-from-file=list &&\n> +\ttest_must_fail git reset --hard --pathspec-from-file=list\n> +'\n> +\n> +test_expect_success 'only touches what was listed' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n> +\tprintf \"fileB.t\\nfileC.t\" | git reset --pathspec-from-file=- &&\n> +\n> +\tverify_state expect.a_bc_d\n> +'\n> +\n> +test_done\n> \n"},{"id":"385460","messageId":"9ca7fa57-c438-7243-6ab1-956d8f132d37@gmail.com","threadId":"52185","inReplyTo":"f4847046896848d3f16bc5f3cb7a26271cefd97c.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 5/5] commit: support the --pathspec-from-file option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-11-05T16:27:25Z","receivedAt":"2019-11-05T16:27:32Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alexandr\n\nOn 04/11/2019 19:26, Alexandr Miloslavskiy via GitGitGadget wrote:\n> From: Alexandr Miloslavskiy \n> \n> This option solves the problem of commandline length limit for UI's\n> built on top of git. Plumbing commands are not always a good fit, for\n> two major reasons:\n> 1) Some UI's serve as assistants that help user run git commands. In\n> this case, replacing familiar commands with plumbing commands will\n> confuse most users.\n> 2) Some UI's have started and grown with porcelain commands. Replacing\n> existing logic with plumbing commands could be cumbersome and prone\n> to various new problems.\n> \n> The new option is designed to behave very close to pathspecs passed in\n> commandline args, so that switching from one to another is simple.\n> \n> The new option allows to read either a specified file or `stdin`.\n> Reading from file is a good way to avoid competing for stdin, and\n> also gives some extra flexibility.\n> \n> Decisions taken for simplicity:\n> 1) The new option is declared incompatible with other options that\n> could use `stdin`.\n> 2) It is not allowed to pass some refspecs in args and others in file.\n> 3) New options do not have shorthands to avoid shorthand conflicts.\n> \n> Also add new '--pathspec-file-null' switch that mirrors '-z' used in\n> various places. Some porcelain commands, such as `git commit`, already\n> use '-z', therefore it needed a new unambiguous name.\n\nIt might be worth tailoring the message to the command rather than \nhaving exactly the same message for commit and reset\n\n> \n> Signed-off-by: Alexandr Miloslavskiy \n> ---\n> Documentation/git-commit.txt | 14 ++++-\n> builtin/commit.c | 25 ++++++--\n> t/t7526-commit-pathspec-file.sh | 107 ++++++++++++++++++++++++++++++++\n> 3 files changed, 141 insertions(+), 5 deletions(-)\n> create mode 100755 t/t7526-commit-pathspec-file.sh\n> \n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index 4341d0e3ab..ec4752298d 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -13,7 +13,8 @@ SYNOPSIS\n> \t [-F | -m ] [--reset-author] [--allow-empty]\n> \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n> \t [--date=] [--cleanup=] [--[no-]status]\n> -\t [-i | -o] [-S[]] [--] [...]\n> +\t [-i | -o] [--pathspec-from-file= [--pathspec-file-null]]\n> +\t [-S[]] [--] [...]\n> \n> DESCRIPTION\n> -----------\n> @@ -277,6 +278,17 @@ FROM UPSTREAM REBASE\" section in linkgit:git-rebase[1].)\n> \tthe last commit without committing changes that have\n> \talready been staged. If used together with `--allow-empty`\n> \tpaths are also not required, and an empty commit will be created.\n> +\t\n> +--pathspec-from-file=::\n> +\tRead `` from `` instead. If `` is exactly `-`\n> +\tthen read from standard input. Pathspecs are separated by LF or\n> +\tCR/LF. Pathspecs can be quoted as explained for the configuration\n> +\tvariable `core.quotePath` (see linkgit:git-config[1]). See also\n> +\t`--pathspec-file-null` and global `--literal-pathspecs`.\n> +\n> +--pathspec-file-null::\n> +\tOnly meaningful with `--pathspec-from-file`. Pathspecs are\n> +\tseparated with NUL character and are not expected to be quoted.\n\nI think my comments from patch 3 about and the option names \napply here as well\n\n> -u[]::\n> --untracked-files[=]::\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index e588bc6ad3..532f305926 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n> static int edit_flag = -1; /* unspecified */\n> static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n> static int config_commit_verbose = -1; /* unspecified */\n> -static int no_post_rewrite, allow_empty_message;\n> +static int no_post_rewrite, allow_empty_message, pathspec_file_null;\n> static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n> -static char *sign_commit;\n> +static char *sign_commit, *pathspec_from_file;\n> \n> /*\n> * The default commit message cleanup mode will remove the lines\n> @@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n> \t\t PATHSPEC_PREFER_FULL,\n> \t\t prefix, argv);\n> \n> +\tif (pathspec_from_file) {\n> +\t\tif (interactive)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n> +\n> +\t\tif (pathspec.nr)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n> +\n> +\t\tparse_pathspec_file(&pathspec, 0,\n> +\t\t\t\t PATHSPEC_PREFER_FULL,\n> +\t\t\t\t prefix, pathspec_from_file, pathspec_file_null);\n> +\t}\n> +\telse if (pathspec_file_null)\n> +\t\tdie(_(\"--pathspec-file-null requires --pathspec-from-file\"));\n> +\n> +\tif (!pathspec.nr && (also || (only && !amend && !allow_empty)))\n> +\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n\nI wonder if there is a way of calling parse_pathspec_file() from \nparse_and_validate_options() instead. Otherwise we end up validating \noptions here instead which is a bit messy.\n\n> \tif (read_cache_preload(&pathspec) < 0)\n> \t\tdie(_(\"index file corrupt\"));\n> \n> @@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n> \n> \tif (also + only + all + interactive > 1)\n> \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n> -\tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n> -\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n> \tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n> \n> \thandle_untracked_files_arg(s);\n> @@ -1535,6 +1550,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BOOL(0, \"amend\", &amend, N_(\"amend previous commit\")),\n> \t\tOPT_BOOL(0, \"no-post-rewrite\", &no_post_rewrite, N_(\"bypass post-rewrite hook\")),\n> \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n> +\t\tOPT_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file, N_(\"read pathspecs from file\")),\n> +\t\tOPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null, N_(\"with --pathspec-from-file, pathspecs are separated with NUL character\")),\n> \t\t/* end commit contents options */\n> \n> \t\tOPT_HIDDEN_BOOL(0, \"allow-empty\", &allow_empty,\n> diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\n> new file mode 100755\n> index 0000000000..c5d68e01e6\n> --- /dev/null\n> +++ b/t/t7526-commit-pathspec-file.sh\n\nThe test comments from patch 3 apply here as well I think.\n\nOverall this series is nicely structured and is looking pretty good\n\nBest Wishes\n\nPhillip\n\n> @@ -0,0 +1,107 @@\n> +#!/bin/sh\n> +\n> +test_description='commit --pathspec-from-file'\n> +\n> +. ./test-lib.sh\n> +\n> +test_tick\n> +\n> +cat > expect.a < +A\tfileA.t\n> +EOF\n> +\n> +cat > expect.ab < +A\tfileA.t\n> +A\tfileB.t\n> +EOF\n> +\n> +cat > expect.bc < +A\tfileB.t\n> +A\tfileC.t\n> +EOF\n> +\n> +test_expect_success setup '\n> +\ttest_commit file0 &&\n> +\tcheckpoint=$(git rev-parse --verify HEAD) &&\n> +\t\n> +\techo A >fileA.t &&\n> +\techo B >fileB.t &&\n> +\techo C >fileC.t &&\n> +\techo D >fileD.t &&\n> +\tgit add fileA.t fileB.t fileC.t fileD.t\n> +'\n> +\n> +restore_checkpoint () {\n> +\tgit reset --soft \"$checkpoint\"\n> +}\n> +\n> +verify_state () {\n> +\tgit diff-tree --no-commit-id --name-status -r HEAD >actual &&\n> +\ttest_cmp \"$1\" actual\n> +}\n> +\n> +test_expect_success '--pathspec-from-file from stdin' '\n> +\trestore_checkpoint &&\n> +\n> +\techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success '--pathspec-from-file from file' '\n> +\trestore_checkpoint &&\n> +\n> +\techo fileA.t >list &&\n> +\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success 'NUL delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tprintf fileA.tQfileB.t | q_to_nul | git commit --pathspec-from-file=- --pathspec-file-null -m \"Commit\" &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'LF delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'CRLF delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tprintf \"fileA.t\\r\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n> +\n> +\tverify_state expect.ab\n> +'\n> +\n> +test_expect_success 'quotes' '\n> +\trestore_checkpoint &&\n> +\n> +\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success 'quotes not compatible with --pathspec-file-null' '\n> +\trestore_checkpoint &&\n> +\n> +\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n> +\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-null -m \"Commit\"\n> +'\n> +\n> +test_expect_success 'only touches what was listed' '\n> +\trestore_checkpoint &&\n> +\n> +\tprintf \"fileB.t\\nfileC.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n> +\n> +\tverify_state expect.bc\n> +'\n> +\n> +test_done\n> \n"},{"id":"385478","messageId":"2145317c-873c-19b9-6ec9-7502656a27ff@syntevo.com","threadId":"52185","inReplyTo":"e34eba95-f413-ddd3-08c6-4f5b2bed3761@gmail.com","subject":"Re: [PATCH 1/5] pathspec: add new function to parse file","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-05T19:14:02Z","receivedAt":"2019-11-05T19:14:08Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 05.11.2019 16:02, Phillip Wood wrote:\n\n>> +    if (!in)\n>> +        die(_(\"could not open '%s' for reading\"), file);\n> \n> die_errno() would give a more informative message here\n\nThanks for the pointer!\n\n>> +            if (unquote_c_style(&unquoted, buf.buf, NULL))\n>> +                die(_(\"line is badly quoted\"));\n> \n> It would be nice to show the offending line in the error message\n\nGood idea.\n\n>> +void parse_pathspec_file(struct pathspec *pathspec,\n>> +            unsigned magic_mask,\n>> +            unsigned flags,\n>> +            const char *prefix,\n>> +            const char *file,\n>> +            int nul_term_line);\n> \n> Do these align with the 's' in \"struct pathspec\" ?\n\nSorry, still struggling with my VS that tries to do crazy things. Will fix.\n"},{"id":"385479","messageId":"af4ecf54-6353-2307-5905-624919e78683@gmail.com","threadId":"52185","inReplyTo":"b93592df-561f-36d4-09ce-e02d570c60da@gmail.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-11-05T19:22:08Z","receivedAt":"2019-11-05T19:22:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Alexandr\n\nOn 05/11/2019 15:03, Phillip Wood wrote:\n> [...]\n>> +test_expect_success 'quotes' '\n>> +    restore_checkpoint &&\n>> +\n>> +    git rm fileA.t &&\n>> +    printf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n>> +\n>> +    verify_state expect.a\n> \n> If I've understood correctly this doesn't test if a path is correctly \n> unquoted, only that it is accepted.\n\nOh I think I've misunderstood. The '\\\\' in unquoted by printf, so git \nsees '\\101' which is A so that is a real file.\n\nSorry for the confusion\n\nPhillip\n\n> \n>> +'\n>> +\n>> +test_expect_success 'quotes not compatible with --pathspec-file-null' '\n>> +    restore_checkpoint &&\n>> +\n>> +    git rm fileA.t &&\n>> +    printf \"\\\"file\\\\101.t\\\"\" >list &&\n>> +    # Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n>> +    git reset --pathspec-from-file=list --pathspec-file-null &&\n>> +\n>> +    test_must_fail verify_state expect.a\n>> +'\n>> +\n>> +test_expect_success '--pathspec-from-file is not compatible with \n>> --soft --hard' '\n> \n> s/--soft --hard/--soft or --hard/\n> \n>> +    restore_checkpoint &&\n>> +\n>> +    git rm fileA.t &&\n>> +    echo fileA.t >list &&\n>> +    test_must_fail git reset --soft --pathspec-from-file=list &&\n>> +    test_must_fail git reset --hard --pathspec-from-file=list\n>> +'\n>> +\n>> +test_expect_success 'only touches what was listed' '\n> \n> s/^/--pathspec-from-file / ?\n> \n> Best Wishes\n> \n> Phillip\n> \n>> +    restore_checkpoint &&\n>> +\n>> +    git rm fileA.t fileB.t fileC.t fileD.t &&\n>> +    printf \"fileB.t\\nfileC.t\" | git reset --pathspec-from-file=- &&\n>> +\n>> +    verify_state expect.a_bc_d\n>> +'\n>> +\n>> +test_done\n>>\n"},{"id":"385482","messageId":"8548babb-db15-3fc8-f99f-678032e476c3@syntevo.com","threadId":"52185","inReplyTo":"b93592df-561f-36d4-09ce-e02d570c60da@gmail.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-05T19:36:31Z","receivedAt":"2019-11-05T19:36:41Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 05.11.2019 16:03, Phillip Wood wrote:\n\n>> The new option allows to read either a specified file or `stdin`.\n>> Reading from file is a good way to avoid competing for stdin, and\n>> also gives some extra flexibility.\n> \n> I think this flexibility is a good idea\n\nThanks for your support :) Previously the opinions were mixed and I was \na bit afraid that this will invoke a new round of discussions.\n\n>> Decisions taken for simplicity:\n>> 1) The new option is declared incompatible with other options that\n>>     could use `stdin`.\n> \n> I'm confused reset does not use stdin does it?\n\nI understand that '--patch' interacts with user via stdin. Will \ndouble-check tomorrow.\n\n>> 2) It is not allowed to pass some refspecs in args and others in file.\n> \n> s/refspecs/pathspecs/ ?\n\nThanks! Not quite used to git speak and mix up things sometimes.\n\n>> Also add new '--pathspec-file-null' switch that mirrors '-z' used in\n>> various places. Some porcelain commands, such as `git commit`, already\n>> use '-z', therefore it needed a new unambiguous name.\n> \n> As the 'lines' in the file are nul terminated perhaps it would be better \n> to call this --pathspec-file-nul or --nul-termination. I think the use \n> of --null to mean nul termination for config was a mistake (for grep it \n> matches what GUN grep does but it's still unfortunate).\n\nOK, will change to '--pathspec-file-nul'\n\n>> +'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] \n>> []\n> \n> --pathspec-file would be shorter and still conveys the intent of the \n> option. Is this line missing a leading space?\n\n'--pathspec-from-file' was kind of suggested instead of '--paths-file' \nby Junio here:\nhttps://public-inbox.org/git/xmqqtv9qr82q.fsf@gitster-ct.c.googlers.com/\n\nso Junio added '-from' to previous writing. Hmm. What do you think, \ntaking Junio's message into account?\n\nAs for whitespaces, sorry, will fix.\n\n>> +'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] \n>> []::\n> \n> Alignment again\n\nWill fix\n\n>> +    These forms reset the index entries for all `` to their\n> \n> The new form does not mention so this could be confusing\n\nShall I replace '' with 'pathspecs' ?\n\n> As we have a separate synopsis line for --pathspec-from-file which does \n> not mention it might be better just to say \"read pathspecs \n> from `` instead of the command line\".\n\nOK\n\n>> +    if (pathspec_from_file) {\n>> +        if (patch_mode)\n>> +            die(_(\"--pathspec-from-file is incompatible with --patch\"));\n> \n> This is sensible as -p is interactive so we wouldn't expect command line \n> length to be an issue\n\nYes, I also thought so. I doubt that user is willing to interactively \ndecide on hundreds of files.\n\n>> +    } else if (pathspec_file_null)\n>> +        die(_(\"--pathspec-file-null requires --pathspec-from-file\"));\n> \n> Style nit: the coding guidelines state that if any branch of an if \n> statement requires braces then all the branches should be braced. This \n> is widely ignored though.\n\nTook this code from a guy who ignored it :) But sure, will change.\n\n> These days we tend to set up the expected files within the relevant test \n> case using <<-\\EOF to allow indentation and disallow substitution \n> (unless it's needed of course)\n\nI'll try to change this.\n\n>> +test_expect_success 'quotes' '\n>> +    restore_checkpoint &&\n>> +\n>> +    git rm fileA.t &&\n>> +    printf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n>> +\n>> +    verify_state expect.a\n> \n> If I've understood correctly this doesn't test if a path is correctly \n> unquoted, only that it is accepted.\n\nIn my understanding, 'verify_state expect.a' should test that it's \ncorrectly understood. Am I wrong?\n\n>> +test_expect_success '--pathspec-from-file is not compatible with \n>> --soft --hard' '\n> \n> s/--soft --hard/--soft or --hard/\n\nGood idea.\n\n>> +test_expect_success 'only touches what was listed' '\n> \n> s/^/--pathspec-from-file / ?\n\nI thought that whole test package is about '--pathspec-from-file' so I'd \nrather not repeat that in every test name. Shall I change that?\n"},{"id":"385483","messageId":"f3a24b43-62ab-0912-bbd8-ae40052bc2ad@syntevo.com","threadId":"52185","inReplyTo":"3cee48dd-15bb-1bec-6d23-d05cce0f4eb1@gmail.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-05T19:37:16Z","receivedAt":"2019-11-05T19:37:21Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 05.11.2019 17:14, Phillip Wood wrote:\n\n>> +        OPT_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file,\n>> +                N_(\"read pathspecs from file\")),\n>> +        OPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null,\n>> +                N_(\"with --pathspec-from-file, pathspecs are \n>> separated with NUL character\")),\n> \n> One thing I forgot to mention before is that if you're going to add \n> these options to a number of commands then maybe it is worth defining a \n> macro for each one in parse-options.h. There are a couple of examples of \n> this at the end of that file.\n\nThanks for the pointer!\n"},{"id":"385484","messageId":"73fb5aa2-6cdb-424a-5696-a2609d74a034@syntevo.com","threadId":"52185","inReplyTo":"9ca7fa57-c438-7243-6ab1-956d8f132d37@gmail.com","subject":"Re: [PATCH 5/5] commit: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-05T19:42:40Z","receivedAt":"2019-11-05T19:42:46Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 05.11.2019 17:27, Phillip Wood wrote:\n\n>> Also add new '--pathspec-file-null' switch that mirrors '-z' used in\n>> various places. Some porcelain commands, such as `git commit`, already\n>> use '-z', therefore it needed a new unambiguous name.\n> \n> It might be worth tailoring the message to the command rather than \n> having exactly the same message for commit and reset\n\nI also was somewhat unhappy about duplication. But I didn't figure how \nto do that correctly. Currently the messages for 'git reset' and 'git \ncommit' are almost identical.\n\nMaybe in 2nd commit I should say something like \"Extend \n`--pathspec-file-null` support to `git commit` (see previous patch for \n`git reset`)\" ?\n\n> I think my comments from patch 3 about and the option names \n> apply here as well\n\nYes, sure, I will try to apply your suggestions to all patches. \nHopefully without forgetting things :)\n\n>> +    if (!pathspec.nr && (also || (only && !amend && !allow_empty)))\n>> +        die(_(\"No paths with --include/--only does not make sense.\"));\n> \n> I wonder if there is a way of calling parse_pathspec_file() from \n> parse_and_validate_options() instead. Otherwise we end up validating \n> options here instead which is a bit messy.\n\nYes, I was also somewhat unhappy about that. I will give it more thought.\n\n> Overall this series is nicely structured and is looking pretty good\n\nThanks, and also thanks for reviewing my patches!\n\n"},{"id":"385522","messageId":"xmqqh83hveiz.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"1740ac7a291cfc81418c2d437201c3373487fa26.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/5] doc: reset: unify description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-06T04:01:56Z","receivedAt":"2019-11-06T04:02:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> optionally modifying index and working tree to match.\n> The ``/`` defaults to `HEAD` in all forms.\n> \n> -'git reset' [-q] [] [--] ...::\n> -\tThis form resets the index entries for all `` to their\n> +'git reset' [-q] [] [--] ...::\n> +\tThis form resets the index entries for all `` to their\n> \tstate at ``. (It does not affect the working tree or\n> \tthe current branch.)\n> +\n> -This means that `git reset ` is the opposite of `git add\n> -`. This command is equivalent to\n> -`git restore [--source=] --staged ...`.\n> +For more details about the syntax, see the 'pathspec' entry\n> +in linkgit:gitglossary[7].\n> +\n> -After running `git reset ` to update the index entry, you can\n> +This means that `git reset ` is the opposite of `git add\n\nIt is good to show which document to read more on , but\ninserting it just before \"This means...\" breaks the flow of thought.\n\nAs we introduce/explain tree-ish and commit used in the synopsis in\nthe ealier part of the description, it probably is a much better\nplace to also introduce/explain pathspec, perhaps like so:\n\n\t In the first and second form, ...\n\t In the third form, ...\n\t The / defaults to HEAD in all forms.\n\t+The is used to limit the paths affected by the\n\t+operation in the first two forms (see the entry for\n\t+'pathspec' in linkgit:gitglossary[7] for more details).\n\n> +This means that `git reset ` is the opposite of `git add\n> +`. This command is equivalent to\n> +`git restore [--source=] --staged ...`.\n\nAny time I see \"... X. This means Y.\" either in the doc or in the\nproposed log message, I wish the author (not you in this case,\nobviously) thought twice about rewriting so that they do not say one\nthing and immediately have to rephrase it, i.e. either just say Y\nwithout saying X, or saying X more clearly without having to say Y.\n\nIn this case, however, I think X and Y are related but both relevant.\nThe subcommand resets the index entries for chosen paths to match\nwhat is in the tree-ish, which is the same as restoring from a tree\nto the index.\n\nIt is not quite the opposite of adding to the index from the working\ntree. In this sequence:\n\n\t$ edit newfile\n\t$ git add newfile\n\nand then further\n\n\t$ edit newfile\n\t$ git add newfile\n\t$ git reset -- newfile\n\nwe are taken back to the state _before_ any of the changes made to\nnewfile (in fact, since HEAD does not have newfile, the resulting\nindex would not know about it, either).\n\nThanks.\n"},{"id":"385532","messageId":"xmqqd0e5vcq9.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"8d9f1fbc18346144a0c866a59891b652dcfe9b7f.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-06T04:40:46Z","receivedAt":"2019-11-06T04:40:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> From: Alexandr Miloslavskiy \n>\n> This option solves the problem of commandline length limit for UI's\n> built on top of git. Plumbing commands are not always a good fit, for\n> two major reasons:\n> 1) Some UI's serve as assistants that help user run git commands. In\n> this case, replacing familiar commands with plumbing commands will\n> confuse most users.\n\n\"UI's that serve as assistants that help user run git commands\" does\nnot have to avoid plumbing commands at all. Only the ones that \"show\"\nthe commands that are run on behalf of the users (perhaps so that the\nusers can learn from such examples?) do, and I think I learned that\nit was your motivating use case from an earlier discussion. Perhaps\n\n\tUIs that help users to formulate git commands to run need to\n\tpresent Porcelain commands to be used, as it is not reasonable\n\tto demonstrate arcane combination of plumbing commands as an\n\texample for their interactive use.\n\nwould probably be more readable without bending what you wanted to\nsay too much?\n\n> 2) Some UI's have started and grown with porcelain commands. Replacing\n> existing logic with plumbing commands could be cumbersome and prone\n> to various new problems.\n\nThere is not a lot of sympathy for such argument ;-)\n\n> 2) It is not allowed to pass some refspecs in args and others in file.\n\nDid you mean refspec, not pathspec?\n\n> 3) New options do not have shorthands to avoid shorthand conflicts.\n>\n> Also add new '--pathspec-file-null' switch that mirrors '-z' used in\n> various places. Some porcelain commands, such as `git commit`, already\n> use '-z', therefore it needed a new unambiguous name.\n\nI do not understand this part. Wouldn't it natural to expect that\n\"-z\", when used with \"--pathspec-from-file\", tell us that the named\nfile is NUL delimited collection of records? What's different about\n\"--pathspec-file-null\" that it is confusing to all it \"-z\"?\n\nI actually do not mind not having \"-z\" and using only\n\"--pathspec-file-null\". A new option with long name that feels\nsimilar to existing \"-z\" but does sufficiently different things so\nthat it needs a different name, however, feels counter-productive\nfor the purpose of using it in the UI that produces commands to be\nlearned by end-users.\n\n> +--pathspec-from-file=::\n> +\tRead `` from `` instead. If `` is exactly `-`\n> +\tthen read from standard input. Pathspecs are separated by LF or\n> +\tCR/LF. Pathspecs can be quoted as explained for the configuration\n\nWhen I invented the terms \"pathspec\", \"refspec\", etc. for this\nproject, I meant them to be collective nouns. A pathspec is a set\nof pathspec elements, each of which is usually given as a separate\nargument on the command line. So \"Read pathspec from file\" is good\n(not \"Read pathspecs from file\").\n\nAnd \"Pathspec elements\" are separated by LF or CR/LF.\n\nA tangent. Since we do not allow NUL in a pathspec element, we do\nnot even need the \"-z\" option. When we read pathspec from a file,\nwe can take any of CRLF, LF or NUL as the separator.\n\nAh, sorry, that would not help very much. With \"-z\" we are allowing\nto express pathspec elements inside which there are embedded LF or\nCR/LF. Sorry about the noise.\n\n> +--pathspec-file-null::\n> +\tOnly meaningful with `--pathspec-from-file`. Pathspecs are\n> +\tseparated with NUL character and are not expected to be quoted.\n\nOK.\n\n> +\tif (pathspec_from_file) {\n> +\t\tif (patch_mode)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with --patch\"));\n> +\n> +\t\tif (pathspec.nr)\n> +\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n\nShouldn't the error message say pathspec arguments instead?\n\n> diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\n> new file mode 100755\n> index 0000000000..cf7f085ad5\n> --- /dev/null\n> +++ b/t/t7107-reset-pathspec-file.sh\n> @@ -0,0 +1,126 @@\n> +#!/bin/sh\n> +\n> +test_description='reset --pathspec-from-file'\n> +\n> +. ./test-lib.sh\n> +\n> +cat > expect.a < + D fileA.t\n> +EOF\n> +\n> +cat > expect.ab < + D fileA.t\n> + D fileB.t\n> +EOF\n> +\n> +cat > expect.a_bc_d < +D fileA.t\n> + D fileB.t\n> + D fileC.t\n> +D fileD.t\n> +EOF\n> +\n> +test_expect_success setup '\n> +\techo A >fileA.t &&\n> +\techo B >fileB.t &&\n> +\techo C >fileC.t &&\n> +\techo D >fileD.t &&\n> +\tgit add . &&\n> +\tgit commit --include . -m \"Commit\" &&\n> +\tcheckpoint=$(git rev-parse --verify HEAD)\n> +'\n> +\n> +restore_checkpoint () {\n> +\tgit reset --hard \"$checkpoint\"\n> +}\n\nHmm, wouldn't it be cleaner to use a lightweight tag or something to\nkeep checkpoint, instead of a variable that is hard to examine when\ntests break and needs debugging?\n\n> +verify_state () {\n> +\tgit status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&\n> +\ttest_cmp \"$1\" actual\n> +}\n> +\n> +test_expect_success '--pathspec-from-file from stdin' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t | git reset --pathspec-from-file=- &&\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success '--pathspec-from-file from file' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t &&\n> +\techo fileA.t >list &&\n> +\tgit reset --pathspec-from-file=list &&\n> +\n> +\tverify_state expect.a\n> +'\n> +\n> +test_expect_success 'NUL delimiters' '\n> +\trestore_checkpoint &&\n> +\n> +\tgit rm fileA.t fileB.t &&\n> +\tprintf fileA.tQfileB.t | q_to_nul | git reset --pathspec-from-file=- --pathspec-file-null &&\n\nThis feeds \"fileA.tfileB.t\" without after \"fileB.t\" to\nthe command. Intended?\n\nRather, perhaps\n\n\tprintf \"%s\\0\" fileA.t fileB.t\n\nwithout q-to-nul, once you use printf anyway?\n\nIf you truly mean \"delimiter\" (as opposed to \"terminator\"),\n\n\tprintf \"fileA.t\\0fileB.t\"\n\ncan also lose \" | q_to_nul\".\n\nThanks.\n"},{"id":"385534","messageId":"xmqq8sotvcar.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"251d06e27f6bc93b190450ae6e1087a3126b5e52.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/5] doc: commit: unify description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-06T04:50:04Z","receivedAt":"2019-11-06T04:50:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> From: Alexandr Miloslavskiy \n>\n> Synchronize it to `git add`, which has a pretty good description.\n> This also better disambiguates ... header.\n\n\"When files are given on...\" is no longer true with this change (it\nwasn't true with the code before this change anyway).\n\n\tWhen pathspec is given on the command line, commit the\n\tcontents of the files that match the pathspec without\n\trecording the changes already added to the index. ...\n\nThe second sentence also says \"these files\", but that can be left\nas-is, since it would refer to \"the files that match ...\" explained\nin the above sentence.\n\n> +For more details about the syntax, see the 'pathspec' entry\n> +in linkgit:gitglossary[7].\n\nI am not sure if we want to repeat this all over the place.\n\nWe do not say \"For details about the syntax, see the\n'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]\" for\nevery command that takes from the command line.\n\nIs your urge to suggest adding this sentence coming from that you\nare much more familiar with than ? In other\nwords, if you were more familiar with Git, would you still be adding\nthis (and not corresponding one for )?\n\n"},{"id":"385535","messageId":"xmqq4kzhvc82.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"f4847046896848d3f16bc5f3cb7a26271cefd97c.1572895605.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 5/5] commit: support the --pathspec-from-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-06T04:51:41Z","receivedAt":"2019-11-06T04:51:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> From: Alexandr Miloslavskiy \n>\n> This option solves the problem of commandline length limit for UI's\n> built on top of git. Plumbing commands are not always a good fit, for\n> two major reasons:\n> 1) Some UI's serve as assistants that help user run git commands. In\n> this case, replacing familiar commands with plumbing commands will\n> confuse most users.\n> 2) Some UI's have started and grown with porcelain commands. Replacing\n> existing logic with plumbing commands could be cumbersome and prone\n> to various new problems.\n\nI think all the comments I made on \"reset\" with these two options,\nboth to the proposed log message and to the patch text, applies to\nthis this step, too, so I won't repeat them.\n\nThanks for working on this topic.\n"},{"id":"385597","messageId":"2dfaccf0d5c7b24f522a9b215f4a6f1f31f85253.1573055478.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","subject":"[PATCH v2 1/6] parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-06T15:51:13Z","receivedAt":"2019-11-06T15:51:24Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSupport for various porcelain commands will arrive via additional\npatches.\n\n`--pathspec-from-file` solves the problem of commandline length limit\nfor UIs built on top of git. Plumbing commands are not always a good\nfit, for two major reasons:\n1) Some UIs show executed commands to user. In this case, porcelain\n commands are expected. One reason for that is letting user learn git\n commands by clicking UI buttons. The other reason is letting user\n study the history of commands in case of any unexpected results. Both\n of these will lose most of their value if UI uses combinations of\n arcane plumbing commands.\n2) Some UIs have started and grown with porcelain commands. Replacing\n existing logic with plumbing commands could be cumbersome and prone\n to various new problems.\n\n`--pathspec-from-file` will behave very close to pathspec passed in\ncommandline args, so that switching from one to another is simple.\n\n`--pathspec-from-file` will read either a specified file or `stdin`\n(when file is exactly \"-\"). Reading from file is a good way to avoid\ncompeting for `stdin`, and also gives some extra flexibility.\n\n`--pathspec-file-nul` switch mirrors `-z` already used in various\nplaces. Some porcelain commands, such as `git commit`, already use\n`-z`, therefore it needed a new unambiguous name.\n\nNew options do not have shorthands to avoid shorthand conflicts. It is\nnot expected that they will be typed in console.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n parse-options.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/parse-options.h b/parse-options.h\nindex 38a33a087e..c6cc01e715 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -330,5 +330,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)\n #define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)\n #define OPT_CLEANUP(v) OPT_STRING(0, \"cleanup\", v, N_(\"mode\"), N_(\"how to strip spaces and #comments from message\"))\n+#define OPT_PATHSPEC_FROM_FILE(v) OPT_FILENAME(0, \"pathspec-from-file\", v, N_(\"read pathspec from file\"))\n+#define OPT_PATHSPEC_FILE_NUL(v) OPT_BOOL(0, \"pathspec-file-nul\", v, N_(\"with --pathspec-from-file, pathspec elements are separated with NUL character\"))\n \n #endif\n-- \ngitgitgadget\n\n"},{"id":"385598","messageId":"96697ba0724caee556f21ada3959c4190d80c0b9.1573055478.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","subject":"[PATCH v2 2/6] pathspec: add new function to parse file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-06T15:51:14Z","receivedAt":"2019-11-06T15:51:25Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis will be used to support the new option '--pathspec-from-file' in\n`git add`, `git-commit`, `git reset` etc.\n\nNote also that we specifically handle CR/LF line endings to support\nWindows better.\n\nTo simplify code, file is first parsed into `argv_array`. This allows\nto avoid refactoring `parse_pathspec()`.\n\nI considered adding `nul_term_line` to `flags` instead, but decided\nthat it doesn't fit there.\n\nThe new code is mostly taken from `cmd_update_index()` and\n`split_mail_conv()`.\n\nCo-authored-by: Johannes Schindelin \nSigned-off-by: Alexandr Miloslavskiy \n---\n pathspec.c | 38 ++++++++++++++++++++++++++++++++++++++\n pathspec.h | 10 ++++++++++\n 2 files changed, 48 insertions(+)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 12c2b322b3..2e9b376318 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -3,6 +3,8 @@\n #include \"dir.h\"\n #include \"pathspec.h\"\n #include \"attr.h\"\n+#include \"argv-array.h\"\n+#include \"quote.h\"\n \n /*\n * Finds which of the given pathspecs match items in the index.\n@@ -613,6 +615,42 @@ void parse_pathspec(struct pathspec *pathspec,\n \t}\n }\n \n+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,\n+\t\t\t unsigned flags, const char *prefix,\n+\t\t\t const char *file, int nul_term_line)\n+{\n+\tstruct argv_array parsed_file = ARGV_ARRAY_INIT;\n+\tstrbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :\n+\t\t\t\t\t\t strbuf_getline;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf unquoted = STRBUF_INIT;\n+\tFILE *in = NULL;\n+\n+\tif (!strcmp(file, \"-\"))\n+\t\tin = stdin;\n+\telse\n+\t\tin = xfopen(file, \"r\");\n+\n+\twhile (getline_fn(&buf, in) != EOF) {\n+\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&unquoted);\n+\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n+\t\t\t\tdie(_(\"line is badly quoted: %s\"), buf.buf);\n+\t\t\tstrbuf_swap(&buf, &unquoted);\n+\t\t}\n+\t\targv_array_push(&parsed_file, buf.buf);\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\n+\tstrbuf_release(&unquoted);\n+\tstrbuf_release(&buf);\n+\tif (in != stdin)\n+\t\tfclose(in);\n+\n+\tparse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);\n+\targv_array_clear(&parsed_file);\n+}\n+\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src)\n {\n \tint i, j;\ndiff --git a/pathspec.h b/pathspec.h\nindex 1c18a2c90c..a27dc81ba2 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,\n \t\t unsigned flags,\n \t\t const char *prefix,\n \t\t const char **args);\n+/*\n+ * Same as parse_pathspec() but uses file as input.\n+ * When 'file' is exactly \"-\" it uses 'stdin' instead.\n+ */\n+void parse_pathspec_file(struct pathspec *pathspec,\n+\t\t\t unsigned magic_mask,\n+\t\t\t unsigned flags,\n+\t\t\t const char *prefix,\n+\t\t\t const char *file,\n+\t\t\t int nul_term_line);\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src);\n void clear_pathspec(struct pathspec *);\n \n-- \ngitgitgadget\n\n"},{"id":"385599","messageId":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.git.1572895605.gitgitgadget@gmail.com","subject":"[PATCH v2 0/6] Add --pathspec-from-file option for reset, commit","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-06T15:51:12Z","receivedAt":"2019-11-06T15:51:28Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Adds --pathspec-from-file option for porcelain commands to avoid commandline\nlength limit.\n\nSo far I implemented it for git commit and git reset, but my goal is to\nsupport other commands as well after these patches are reviewed.\n\nThe patches are based on the following discussions:\nhttps://public-inbox.org/git/c3be6eff-365b-96b8-16d2-0528612fc1fc@syntevo.com/T/#u\nThere, --stdin-paths was suggested.\nhttps://public-inbox.org/git/a38bc928-7ccd-e2d9-b89b-23298e9fa95d@syntevo.com/T/#u\nThere, --stdin-paths was extended to --paths-file.\nhttps://public-inbox.org/git/pull.133.git.gitgitgadget@gmail.com/\nhttps://github.com/gitgitgadget/git/pull/133Patch from Johannes, I used it\nas base of my patch. There, --pathspec-from-file is suggested in discussion.\n\nMajor changes compared to patch from Johannes:\n\n1) --pathspec-from-file allows to read file, not just stdin\n\n2) --literal-pathspecs should be honored. This is a good improvement because\nit keeps --pathspec-from-file much closer to passing args on commandline.\nSince the goal of the new option is to avoid commandline length limit, both\nbehaviors should be close to each other.\n\n3) Patches are designed with all other git commands in mind\n\nChanges from V1: A lot of small changes here and there, in accordance with\nreview suggestions.\n\nAlexandr Miloslavskiy (6):\n parse-options.h: add new options `--pathspec-from-file`,\n `--pathspec-file-nul`\n pathspec: add new function to parse file\n doc: reset: unify description\n reset: support the `--pathspec-from-file` option\n doc: commit: unify description\n commit: support the --pathspec-from-file option\n\n Documentation/git-commit.txt | 29 ++++--\n Documentation/git-reset.txt | 37 +++++---\n builtin/commit.c | 25 +++++-\n builtin/reset.c | 25 +++++-\n parse-options.h | 2 +\n pathspec.c | 38 ++++++++\n pathspec.h | 10 +++\n t/t7107-reset-pathspec-file.sh | 155 ++++++++++++++++++++++++++++++++\n t/t7526-commit-pathspec-file.sh | 130 +++++++++++++++++++++++++++\n 9 files changed, 425 insertions(+), 26 deletions(-)\n create mode 100755 t/t7107-reset-pathspec-file.sh\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/445\n\nRange-diff vs v1:\n\n -: ---------- > 1: 2dfaccf0d5 parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`\n 1: 52e7a84a2e ! 2: 96697ba072 pathspec: add new function to parse file\n @@ -50,16 +50,13 @@\n +\tif (!strcmp(file, \"-\"))\n +\t\tin = stdin;\n +\telse\n -+\t\tin = fopen(file, \"r\");\n -+\n -+\tif (!in)\n -+\t\tdie(_(\"could not open '%s' for reading\"), file);\n ++\t\tin = xfopen(file, \"r\");\n +\n +\twhile (getline_fn(&buf, in) != EOF) {\n +\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n +\t\t\tstrbuf_reset(&unquoted);\n +\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n -+\t\t\t\tdie(_(\"line is badly quoted\"));\n ++\t\t\t\tdie(_(\"line is badly quoted: %s\"), buf.buf);\n +\t\t\tstrbuf_swap(&buf, &unquoted);\n +\t\t}\n +\t\targv_array_push(&parsed_file, buf.buf);\n @@ -91,11 +88,11 @@\n + * When 'file' is exactly \"-\" it uses 'stdin' instead.\n + */\n +void parse_pathspec_file(struct pathspec *pathspec,\n -+\t\t unsigned magic_mask,\n -+\t\t unsigned flags,\n -+\t\t const char *prefix,\n -+\t\t const char *file,\n -+\t\t int nul_term_line);\n ++\t\t\t unsigned magic_mask,\n ++\t\t\t unsigned flags,\n ++\t\t\t const char *prefix,\n ++\t\t\t const char *file,\n ++\t\t\t int nul_term_line);\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src);\n void clear_pathspec(struct pathspec *);\n \n 2: 1740ac7a29 ! 3: f961a5155a doc: reset: unify description\n @@ -21,8 +21,11 @@\n \n DESCRIPTION\n @@\n + In the third form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n ++The is used to limit the paths affected by the operation\n ++(see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n \n -'git reset' [-q] [] [--] ...::\n -\tThis form resets the index entries for all `` to their\n @@ -34,14 +37,11 @@\n -This means that `git reset ` is the opposite of `git add\n -`. This command is equivalent to\n -`git restore [--source=] --staged ...`.\n -+For more details about the syntax, see the 'pathspec' entry\n -+in linkgit:gitglossary[7].\n - +\n --After running `git reset ` to update the index entry, you can\n +This means that `git reset ` is the opposite of `git add\n +`. This command is equivalent to\n +`git restore [--source=] --staged ...`.\n -++\n + +\n +-After running `git reset ` to update the index entry, you can\n +After running `git reset ` to update the index entry, you can\n use linkgit:git-restore[1] to check the contents out of the index to\n the working tree. Alternatively, using linkgit:git-restore[1]\n @@ -54,10 +54,18 @@\n \tInteractively select hunks in the difference between the index\n \tand `` (defaults to `HEAD`). The chosen hunks are applied\n \tin reverse to the index.\n - +\n -+For more details about the syntax, see the 'pathspec' entry\n -+in linkgit:gitglossary[7].\n -++\n - This means that `git reset -p` is the opposite of `git add -p`, i.e.\n - you can use it to selectively reset hunks. See the ``Interactive Mode''\n - section of linkgit:git-add[1] to learn how to operate the `--patch` mode.\n +\n + diff --git a/builtin/reset.c b/builtin/reset.c\n + --- a/builtin/reset.c\n + +++ b/builtin/reset.c\n +@@\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 --patch [] [--] [...]\"),\n ++\tN_(\"git reset [-q] [] [--] ...\"),\n ++\tN_(\"git reset --patch [] [--] [...]\"),\n + \tNULL\n + };\n + \n 3: 8d9f1fbc18 ! 4: d72d4f16b5 reset: support the --pathspec-from-file option\n @@ -1,33 +1,13 @@\n Author: Alexandr Miloslavskiy \n \n - reset: support the --pathspec-from-file option\n -\n - This option solves the problem of commandline length limit for UI's\n - built on top of git. Plumbing commands are not always a good fit, for\n - two major reasons:\n - 1) Some UI's serve as assistants that help user run git commands. In\n - this case, replacing familiar commands with plumbing commands will\n - confuse most users.\n - 2) Some UI's have started and grown with porcelain commands. Replacing\n - existing logic with plumbing commands could be cumbersome and prone\n - to various new problems.\n -\n - The new option is designed to behave very close to pathspecs passed in\n - commandline args, so that switching from one to another is simple.\n -\n - The new option allows to read either a specified file or `stdin`.\n - Reading from file is a good way to avoid competing for stdin, and\n - also gives some extra flexibility.\n + reset: support the `--pathspec-from-file` option\n \n Decisions taken for simplicity:\n - 1) The new option is declared incompatible with other options that\n - could use `stdin`.\n - 2) It is not allowed to pass some refspecs in args and others in file.\n - 3) New options do not have shorthands to avoid shorthand conflicts.\n -\n - Also add new '--pathspec-file-null' switch that mirrors '-z' used in\n - various places. Some porcelain commands, such as `git commit`, already\n - use '-z', therefore it needed a new unambiguous name.\n + 1) For now, `--pathspec-from-file` is declared incompatible with\n + `--patch`, even when is not `stdin`. Such use case it not\n + really expected. Also, it is harder to support in `git commit`, so\n + I decided to make it incompatible in all places.\n + 2) It is not allowed to pass pathspec in both args and file.\n \n Co-authored-by: Johannes Schindelin \n Signed-off-by: Alexandr Miloslavskiy \n @@ -39,7 +19,7 @@\n --------\n [verse]\n 'git reset' [-q] [] [--] ...\n -+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] []\n ++'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []\n 'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n @@ -51,11 +31,13 @@\n +In the last form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n + The is used to limit the paths affected by the operation\n + (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n \n 'git reset' [-q] [] [--] ...::\n -\tThis form resets the index entries for all `` to their\n -+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-null]] []::\n -+\tThese forms reset the index entries for all `` to their\n ++'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []::\n ++\tThese forms reset the index entries matching pathspec to their\n \tstate at ``. (It does not affect the working tree or\n \tthe current branch.)\n +\n @@ -64,14 +46,15 @@\n \toverride the default behavior.\n \n +--pathspec-from-file=::\n -+\tRead `` from `` instead. If `` is exactly `-`\n -+\tthen read from standard input. Pathspecs are separated by LF or\n -+\tCR/LF. Pathspecs can be quoted as explained for the configuration\n -+\tvariable `core.quotePath` (see linkgit:git-config[1]). See also\n -+\t`--pathspec-file-null` and global `--literal-pathspecs`.\n -+\n -+--pathspec-file-null::\n -+\tOnly meaningful with `--pathspec-from-file`. Pathspecs are\n ++\tPathspec is passed in `` instead of commandline args. If\n ++\t`` is exactly `-` then standard input is used. Pathspec\n ++\telements are separated by LF or CR/LF. Pathspec elements can be\n ++\tquoted as explained for the configuration variable `core.quotePath`\n ++\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n ++\tglobal `--literal-pathspecs`.\n ++\n ++--pathspec-file-nul::\n ++\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n +\tseparated with NUL character and are not expected to be quoted.\n \n EXAMPLES\n @@ -83,9 +66,9 @@\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] [--pathspec-from-file [--pathspec-file-null]] []\"),\n - \tN_(\"git reset --patch [] [--] [...]\"),\n + \tN_(\"git reset [-q] [] [--] ...\"),\n ++\tN_(\"git reset [-q] [--pathspec-from-file [--pathspec-file-nul]] []\"),\n + \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n @@\n @@ -94,7 +77,7 @@\n \tint reset_type = NONE, update_ref_status = 0, quiet = 0;\n -\tint patch_mode = 0, unborn;\n -\tconst char *rev;\n -+\tint patch_mode = 0, pathspec_file_null = 0, unborn;\n ++\tint patch_mode = 0, pathspec_file_nul = 0, unborn;\n +\tconst char *rev, *pathspec_from_file = NULL;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n @@ -103,10 +86,8 @@\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_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file,\n -+\t\t\t\tN_(\"read pathspecs from file\")),\n -+\t\tOPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null,\n -+\t\t\t\tN_(\"with --pathspec-from-file, pathspecs are separated with NUL character\")),\n ++\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n ++\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n \t\tOPT_END()\n \t};\n \n @@ -119,13 +100,14 @@\n +\t\t\tdie(_(\"--pathspec-from-file is incompatible with --patch\"));\n +\n +\t\tif (pathspec.nr)\n -+\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n ++\t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n +\n +\t\tparse_pathspec_file(&pathspec, 0,\n +\t\t\t\t PATHSPEC_PREFER_FULL,\n -+\t\t\t\t prefix, pathspec_from_file, pathspec_file_null);\n -+\t} else if (pathspec_file_null)\n -+\t\tdie(_(\"--pathspec-file-null requires --pathspec-from-file\"));\n ++\t\t\t\t prefix, pathspec_from_file, pathspec_file_nul);\n ++\t} else if (pathspec_file_nul) {\n ++\t\tdie(_(\"--pathspec-file-nul requires --pathspec-from-file\"));\n ++\t}\n +\n \tunborn = !strcmp(rev, \"HEAD\") && get_oid(\"HEAD\", &oid);\n \tif (unborn) {\n @@ -142,21 +124,7 @@\n +\n +. ./test-lib.sh\n +\n -+cat > expect.a < expect.ab < expect.a_bc_d <fileA.t &&\n @@ -165,16 +133,16 @@\n +\techo D >fileD.t &&\n +\tgit add . &&\n +\tgit commit --include . -m \"Commit\" &&\n -+\tcheckpoint=$(git rev-parse --verify HEAD)\n ++\tgit tag checkpoint\n +'\n +\n +restore_checkpoint () {\n -+\tgit reset --hard \"$checkpoint\"\n ++\tgit reset --hard checkpoint\n +}\n +\n -+verify_state () {\n ++verify_expect () {\n +\tgit status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&\n -+\ttest_cmp \"$1\" actual\n ++\ttest_cmp expect actual\n +}\n +\n +test_expect_success '--pathspec-from-file from stdin' '\n @@ -183,7 +151,10 @@\n +\tgit rm fileA.t &&\n +\techo fileA.t | git reset --pathspec-from-file=- &&\n +\n -+\tverify_state expect.a\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success '--pathspec-from-file from file' '\n @@ -193,34 +164,62 @@\n +\techo fileA.t >list &&\n +\tgit reset --pathspec-from-file=list &&\n +\n -+\tverify_state expect.a\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'NUL delimiters' '\n +\trestore_checkpoint &&\n +\n +\tgit rm fileA.t fileB.t &&\n -+\tprintf fileA.tQfileB.t | q_to_nul | git reset --pathspec-from-file=- --pathspec-file-null &&\n ++\tprintf \"fileA.t\\0fileB.t\\0\" | git reset --pathspec-from-file=- --pathspec-file-nul &&\n +\n -+\tverify_state expect.ab\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\t D fileB.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'LF delimiters' '\n +\trestore_checkpoint &&\n +\n +\tgit rm fileA.t fileB.t &&\n ++\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n ++\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\t D fileB.t\n ++\tEOF\n ++\tverify_expect\n ++'\n ++\n ++test_expect_success 'no trailing delimiter' '\n ++\trestore_checkpoint &&\n ++\n ++\tgit rm fileA.t fileB.t &&\n +\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n +\n -+\tverify_state expect.ab\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\t D fileB.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'CRLF delimiters' '\n +\trestore_checkpoint &&\n +\n +\tgit rm fileA.t fileB.t &&\n -+\tprintf \"fileA.t\\r\\nfileB.t\" | git reset --pathspec-from-file=- &&\n ++\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n +\n -+\tverify_state expect.ab\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\t D fileB.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'quotes' '\n @@ -229,21 +228,27 @@\n +\tgit rm fileA.t &&\n +\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n +\n -+\tverify_state expect.a\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n -+test_expect_success 'quotes not compatible with --pathspec-file-null' '\n ++test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n +\trestore_checkpoint &&\n +\n +\tgit rm fileA.t &&\n +\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n +\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n -+\tgit reset --pathspec-from-file=list --pathspec-file-null &&\n ++\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n +\t\n -+\ttest_must_fail verify_state expect.a\n ++\tcat >expect <<-\\EOF &&\n ++\t D fileA.t\n ++\tEOF\n ++\ttest_must_fail verify_expect\n +'\n +\n -+test_expect_success '--pathspec-from-file is not compatible with --soft --hard' '\n ++test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n +\trestore_checkpoint &&\n +\n +\tgit rm fileA.t &&\n @@ -256,9 +261,15 @@\n +\trestore_checkpoint &&\n +\n +\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n -+\tprintf \"fileB.t\\nfileC.t\" | git reset --pathspec-from-file=- &&\n -+\n -+\tverify_state expect.a_bc_d\n ++\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n ++\n ++\tcat >expect <<-\\EOF &&\n ++\tD fileA.t\n ++\t D fileB.t\n ++\t D fileC.t\n ++\tD fileD.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_done\n 4: 251d06e27f ! 5: 20c4495fd3 doc: commit: unify description\n @@ -24,12 +24,16 @@\n \tDo not interpret any more arguments as options.\n \n -...::\n +-\tWhen files are given on the command line, the command\n +-\tcommits the contents of the named files, without\n +-\trecording the changes already staged. The contents of\n +-\tthese files are also staged for the next commit on top\n +-\tof what have been staged before.\n +...::\n - \tWhen files are given on the command line, the command\n - \tcommits the contents of the named files, without\n - \trecording the changes already staged. The contents of\n - \tthese files are also staged for the next commit on top\n - \tof what have been staged before.\n ++\tWhen pathspec is given on the command line, commit the contents of\n ++\tthe files that match the pathspec without recording the changes\n ++\talready added to the index. The contents of these files are also\n ++\tstaged for the next commit on top of what have been staged before.\n ++\n +For more details about the syntax, see the 'pathspec' entry\n +in linkgit:gitglossary[7].\n 5: f484704689 ! 6: cb5fc9b14d commit: support the --pathspec-from-file option\n @@ -2,32 +2,12 @@\n \n commit: support the --pathspec-from-file option\n \n - This option solves the problem of commandline length limit for UI's\n - built on top of git. Plumbing commands are not always a good fit, for\n - two major reasons:\n - 1) Some UI's serve as assistants that help user run git commands. In\n - this case, replacing familiar commands with plumbing commands will\n - confuse most users.\n - 2) Some UI's have started and grown with porcelain commands. Replacing\n - existing logic with plumbing commands could be cumbersome and prone\n - to various new problems.\n -\n - The new option is designed to behave very close to pathspecs passed in\n - commandline args, so that switching from one to another is simple.\n -\n - The new option allows to read either a specified file or `stdin`.\n - Reading from file is a good way to avoid competing for stdin, and\n - also gives some extra flexibility.\n -\n Decisions taken for simplicity:\n - 1) The new option is declared incompatible with other options that\n - could use `stdin`.\n - 2) It is not allowed to pass some refspecs in args and others in file.\n - 3) New options do not have shorthands to avoid shorthand conflicts.\n -\n - Also add new '--pathspec-file-null' switch that mirrors '-z' used in\n - various places. Some porcelain commands, such as `git commit`, already\n - use '-z', therefore it needed a new unambiguous name.\n + 1) For now, `--pathspec-from-file` is declared incompatible with\n + `--interactive/--patch`, even when is not `stdin`. Such use\n + case it not really expected. Also, it would require changes to\n + `interactive_add()`.\n + 2) It is not allowed to pass pathspec in both args and file.\n \n Signed-off-by: Alexandr Miloslavskiy \n \n @@ -39,7 +19,7 @@\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n -\t [-i | -o] [-S[]] [--] [...]\n -+\t [-i | -o] [--pathspec-from-file= [--pathspec-file-null]]\n ++\t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n +\t [-S[]] [--] [...]\n \n DESCRIPTION\n @@ -50,14 +30,15 @@\n \tpaths are also not required, and an empty commit will be created.\n +\t\n +--pathspec-from-file=::\n -+\tRead `` from `` instead. If `` is exactly `-`\n -+\tthen read from standard input. Pathspecs are separated by LF or\n -+\tCR/LF. Pathspecs can be quoted as explained for the configuration\n -+\tvariable `core.quotePath` (see linkgit:git-config[1]). See also\n -+\t`--pathspec-file-null` and global `--literal-pathspecs`.\n -+\n -+--pathspec-file-null::\n -+\tOnly meaningful with `--pathspec-from-file`. Pathspecs are\n ++\tPathspec is passed in `` instead of commandline args. If\n ++\t`` is exactly `-` then standard input is used. Pathspec\n ++\telements are separated by LF or CR/LF. Pathspec elements can be\n ++\tquoted as explained for the configuration variable `core.quotePath`\n ++\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n ++\tglobal `--literal-pathspecs`.\n ++\n ++--pathspec-file-nul::\n ++\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n +\tseparated with NUL character and are not expected to be quoted.\n \n -u[]::\n @@ -71,7 +52,7 @@\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n -static int no_post_rewrite, allow_empty_message;\n -+static int no_post_rewrite, allow_empty_message, pathspec_file_null;\n ++static int no_post_rewrite, allow_empty_message, pathspec_file_nul;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n -static char *sign_commit;\n +static char *sign_commit, *pathspec_from_file;\n @@ -87,14 +68,14 @@\n +\t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n +\n +\t\tif (pathspec.nr)\n -+\t\t\tdie(_(\"--pathspec-from-file is incompatible with path arguments\"));\n ++\t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n +\n +\t\tparse_pathspec_file(&pathspec, 0,\n +\t\t\t\t PATHSPEC_PREFER_FULL,\n -+\t\t\t\t prefix, pathspec_from_file, pathspec_file_null);\n ++\t\t\t\t prefix, pathspec_from_file, pathspec_file_nul);\n ++\t} else if (pathspec_file_nul) {\n ++\t\tdie(_(\"--pathspec-file-nul requires --pathspec-from-file\"));\n +\t}\n -+\telse if (pathspec_file_null)\n -+\t\tdie(_(\"--pathspec-file-null requires --pathspec-from-file\"));\n +\n +\tif (!pathspec.nr && (also || (only && !amend && !allow_empty)))\n +\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n @@ -115,8 +96,8 @@\n \t\tOPT_BOOL(0, \"amend\", &amend, N_(\"amend previous commit\")),\n \t\tOPT_BOOL(0, \"no-post-rewrite\", &no_post_rewrite, N_(\"bypass post-rewrite hook\")),\n \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n -+\t\tOPT_FILENAME(0, \"pathspec-from-file\", &pathspec_from_file, N_(\"read pathspecs from file\")),\n -+\t\tOPT_BOOL(0, \"pathspec-file-null\", &pathspec_file_null, N_(\"with --pathspec-from-file, pathspecs are separated with NUL character\")),\n ++\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n ++\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n \t\t/* end commit contents options */\n \n \t\tOPT_HIDDEN_BOOL(0, \"allow-empty\", &allow_empty,\n @@ -134,23 +115,9 @@\n +\n +test_tick\n +\n -+cat > expect.a < expect.ab < expect.bc <fileA.t &&\n +\techo B >fileB.t &&\n @@ -160,12 +127,12 @@\n +'\n +\n +restore_checkpoint () {\n -+\tgit reset --soft \"$checkpoint\"\n ++\tgit reset --soft checkpoint\n +}\n +\n -+verify_state () {\n ++verify_expect () {\n +\tgit diff-tree --no-commit-id --name-status -r HEAD >actual &&\n -+\ttest_cmp \"$1\" actual\n ++\ttest_cmp expect actual\n +}\n +\n +test_expect_success '--pathspec-from-file from stdin' '\n @@ -173,7 +140,10 @@\n +\n +\techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n +\n -+\tverify_state expect.a\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileA.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success '--pathspec-from-file from file' '\n @@ -182,31 +152,58 @@\n +\techo fileA.t >list &&\n +\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n +\n -+\tverify_state expect.a\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileA.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'NUL delimiters' '\n +\trestore_checkpoint &&\n +\n -+\tprintf fileA.tQfileB.t | q_to_nul | git commit --pathspec-from-file=- --pathspec-file-null -m \"Commit\" &&\n ++\tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n +\n -+\tverify_state expect.ab\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileA.t\n ++\tA\tfileB.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'LF delimiters' '\n +\trestore_checkpoint &&\n +\n ++\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n ++\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileA.t\n ++\tA\tfileB.t\n ++\tEOF\n ++\tverify_expect\n ++'\n ++\n ++test_expect_success 'no trailing delimiter' '\n ++\trestore_checkpoint &&\n ++\n +\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n +\n -+\tverify_state expect.ab\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileA.t\n ++\tA\tfileB.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'CRLF delimiters' '\n +\trestore_checkpoint &&\n +\n -+\tprintf \"fileA.t\\r\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n ++\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n +\n -+\tverify_state expect.ab\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileA.t\n ++\tA\tfileB.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_expect_success 'quotes' '\n @@ -214,22 +211,29 @@\n +\n +\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n +\n -+\tverify_state expect.a\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileA.t\n ++\tEOF\n ++\tverify_expect expect\n +'\n +\n -+test_expect_success 'quotes not compatible with --pathspec-file-null' '\n ++test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n +\trestore_checkpoint &&\n +\n +\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n -+\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-null -m \"Commit\"\n ++\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n +'\n +\n +test_expect_success 'only touches what was listed' '\n +\trestore_checkpoint &&\n +\n -+\tprintf \"fileB.t\\nfileC.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n ++\tprintf \"fileB.t\\nfileC.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n +\n -+\tverify_state expect.bc\n ++\tcat >expect <<-\\EOF &&\n ++\tA\tfileB.t\n ++\tA\tfileC.t\n ++\tEOF\n ++\tverify_expect\n +'\n +\n +test_done\n\n-- \ngitgitgadget\n"},{"id":"385600","messageId":"f961a5155a5ec1590652ce7b5a549eead71930e1.1573055478.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","subject":"[PATCH v2 3/6] doc: reset: unify description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-06T15:51:15Z","receivedAt":"2019-11-06T15:51:28Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSynchronize it to `git add`, which has a pretty good description.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-reset.txt | 20 +++++++++++---------\n builtin/reset.c | 4 ++--\n 2 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 97e0544d9e..f2ccb1426c 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state\n SYNOPSIS\n --------\n [verse]\n-'git reset' [-q] [] [--] ...\n-'git reset' (--patch | -p) [] [--] [...]\n+'git reset' [-q] [] [--] ...\n+'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n@@ -18,24 +18,26 @@ In the first and second form, copy entries from `` to the index.\n In the third form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n+The is used to limit the paths affected by the operation\n+(see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n \n-'git reset' [-q] [] [--] ...::\n-\tThis form resets the index entries for all `` to their\n+'git reset' [-q] [] [--] ...::\n+\tThis form resets the index entries for all `` to their\n \tstate at ``. (It does not affect the working tree or\n \tthe current branch.)\n +\n-This means that `git reset ` is the opposite of `git add\n-`. This command is equivalent to\n-`git restore [--source=] --staged ...`.\n+This means that `git reset ` is the opposite of `git add\n+`. This command is equivalent to\n+`git restore [--source=] --staged ...`.\n +\n-After running `git reset ` to update the index entry, you can\n+After running `git reset ` to update the index entry, you can\n use linkgit:git-restore[1] to check the contents out of the index to\n the working tree. Alternatively, using linkgit:git-restore[1]\n and specifying a commit with `--source`, you\n can copy the contents of a path out of a commit to the index and to the\n working tree in one go.\n \n-'git reset' (--patch | -p) [] [--] [...]::\n+'git reset' (--patch | -p) [] [--] [...]::\n \tInteractively select hunks in the difference between the index\n \tand `` (defaults to `HEAD`). The chosen hunks are applied\n \tin reverse to the index.\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fdd572168b..9291c0fd72 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -30,8 +30,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] [] [--] ...\"),\n-\tN_(\"git reset --patch [] [--] [...]\"),\n+\tN_(\"git reset [-q] [] [--] ...\"),\n+\tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n \n-- \ngitgitgadget\n\n"},{"id":"385602","messageId":"cb5fc9b14dbe81703a7fc61b56f5c68c4a49ebc2.1573055478.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","subject":"[PATCH v2 6/6] commit: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-06T15:51:18Z","receivedAt":"2019-11-06T15:51:29Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nDecisions taken for simplicity:\n1) For now, `--pathspec-from-file` is declared incompatible with\n `--interactive/--patch`, even when is not `stdin`. Such use\n case it not really expected. Also, it would require changes to\n `interactive_add()`.\n2) It is not allowed to pass pathspec in both args and file.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 15 +++-\n builtin/commit.c | 25 +++++-\n t/t7526-commit-pathspec-file.sh | 130 ++++++++++++++++++++++++++++++++\n 3 files changed, 165 insertions(+), 5 deletions(-)\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex 915c212a0d..bbd53959bd 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,8 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n+\t [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -277,6 +278,18 @@ FROM UPSTREAM REBASE\" section in linkgit:git-rebase[1].)\n \tthe last commit without committing changes that have\n \talready been staged. If used together with `--allow-empty`\n \tpaths are also not required, and an empty commit will be created.\n+\t\n+--pathspec-from-file=::\n+\tPathspec is passed in `` instead of commandline args. If\n+\t`` is exactly `-` then standard input is used. Pathspec\n+\telements are separated by LF or CR/LF. Pathspec elements can be\n+\tquoted as explained for the configuration variable `core.quotePath`\n+\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n+\tglobal `--literal-pathspecs`.\n+\n+--pathspec-file-nul::\n+\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n+\tseparated with NUL character and are not expected to be quoted.\n \n -u[]::\n --untracked-files[=]::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e588bc6ad3..ed40729355 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n-static int no_post_rewrite, allow_empty_message;\n+static int no_post_rewrite, allow_empty_message, pathspec_file_nul;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n-static char *sign_commit;\n+static char *sign_commit, *pathspec_from_file;\n \n /*\n * The default commit message cleanup mode will remove the lines\n@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\t PATHSPEC_PREFER_FULL,\n \t\t prefix, argv);\n \n+\tif (pathspec_from_file) {\n+\t\tif (interactive)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n+\n+\t\tparse_pathspec_file(&pathspec, 0,\n+\t\t\t\t PATHSPEC_PREFER_FULL,\n+\t\t\t\t prefix, pathspec_from_file, pathspec_file_nul);\n+\t} else if (pathspec_file_nul) {\n+\t\tdie(_(\"--pathspec-file-nul requires --pathspec-from-file\"));\n+\t}\n+\n+\tif (!pathspec.nr && (also || (only && !amend && !allow_empty)))\n+\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n+\n \tif (read_cache_preload(&pathspec) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (also + only + all + interactive > 1)\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n-\tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n-\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n \tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n \n \thandle_untracked_files_arg(s);\n@@ -1535,6 +1550,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"amend\", &amend, N_(\"amend previous commit\")),\n \t\tOPT_BOOL(0, \"no-post-rewrite\", &no_post_rewrite, N_(\"bypass post-rewrite hook\")),\n \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n \t\t/* end commit contents options */\n \n \t\tOPT_HIDDEN_BOOL(0, \"allow-empty\", &allow_empty,\ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nnew file mode 100755\nindex 0000000000..4fdf206dd0\n--- /dev/null\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -0,0 +1,130 @@\n+#!/bin/sh\n+\n+test_description='commit --pathspec-from-file'\n+\n+. ./test-lib.sh\n+\n+test_tick\n+\n+test_expect_success setup '\n+\ttest_commit file0 &&\n+\tgit tag checkpoint &&\n+\t\n+\techo A >fileA.t &&\n+\techo B >fileB.t &&\n+\techo C >fileC.t &&\n+\techo D >fileD.t &&\n+\tgit add fileA.t fileB.t fileC.t fileD.t\n+'\n+\n+restore_checkpoint () {\n+\tgit reset --soft checkpoint\n+}\n+\n+verify_expect () {\n+\tgit diff-tree --no-commit-id --name-status -r HEAD >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success '--pathspec-from-file from stdin' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success '--pathspec-from-file from file' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t >list &&\n+\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'quotes' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect expect\n+'\n+\n+test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n+'\n+\n+test_expect_success 'only touches what was listed' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileB.t\n+\tA\tfileC.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"385601","messageId":"20c4495fd31110c56cca84385bee6d1ab4a6f33e.1573055478.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","subject":"[PATCH v2 5/6] doc: commit: unify description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-06T15:51:17Z","receivedAt":"2019-11-06T15:51:30Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSynchronize it to `git add`, which has a pretty good description.\nThis also better disambiguates ... header.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex afa7b75a23..915c212a0d 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -345,12 +345,14 @@ changes to tracked files.\n \\--::\n \tDo not interpret any more arguments as options.\n \n-...::\n-\tWhen files are given on the command line, the command\n-\tcommits the contents of the named files, without\n-\trecording the changes already staged. The contents of\n-\tthese files are also staged for the next commit on top\n-\tof what have been staged before.\n+...::\n+\tWhen pathspec is given on the command line, commit the contents of\n+\tthe files that match the pathspec without recording the changes\n+\talready added to the index. The contents of these files are also\n+\tstaged for the next commit on top of what have been staged before.\n++\n+For more details about the syntax, see the 'pathspec' entry\n+in linkgit:gitglossary[7].\n \n :git-commit: 1\n include::date-formats.txt[]\n-- \ngitgitgadget\n\n"},{"id":"385603","messageId":"d72d4f16b5f77df7cfb4379c9464b2a89cd703f8.1573055478.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","subject":"[PATCH v2 4/6] reset: support the `--pathspec-from-file` option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-06T15:51:16Z","receivedAt":"2019-11-06T15:51:32Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nDecisions taken for simplicity:\n1) For now, `--pathspec-from-file` is declared incompatible with\n `--patch`, even when is not `stdin`. Such use case it not\n really expected. Also, it is harder to support in `git commit`, so\n I decided to make it incompatible in all places.\n2) It is not allowed to pass pathspec in both args and file.\n\nCo-authored-by: Johannes Schindelin \nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-reset.txt | 19 +++-\n builtin/reset.c | 21 ++++-\n t/t7107-reset-pathspec-file.sh | 155 +++++++++++++++++++++++++++++++++\n 3 files changed, 190 insertions(+), 5 deletions(-)\n create mode 100755 t/t7107-reset-pathspec-file.sh\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex f2ccb1426c..d2cd0af6a4 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -9,20 +9,22 @@ SYNOPSIS\n --------\n [verse]\n 'git reset' [-q] [] [--] ...\n+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []\n 'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n -----------\n-In the first and second form, copy entries from `` to the index.\n-In the third form, set the current branch head (`HEAD`) to ``,\n+In the first three forms, copy entries from `` to the index.\n+In the last form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n The is used to limit the paths affected by the operation\n (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n \n 'git reset' [-q] [] [--] ...::\n-\tThis form resets the index entries for all `` to their\n+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []::\n+\tThese forms reset the index entries matching pathspec to their\n \tstate at ``. (It does not affect the working tree or\n \tthe current branch.)\n +\n@@ -103,6 +105,17 @@ OPTIONS\n \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n \toverride the default behavior.\n \n+--pathspec-from-file=::\n+\tPathspec is passed in `` instead of commandline args. If\n+\t`` is exactly `-` then standard input is used. Pathspec\n+\telements are separated by LF or CR/LF. Pathspec elements can be\n+\tquoted as explained for the configuration variable `core.quotePath`\n+\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n+\tglobal `--literal-pathspecs`.\n+\n+--pathspec-file-nul::\n+\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n+\tseparated with NUL character and are not expected to be quoted.\n \n EXAMPLES\n --------\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 9291c0fd72..246bf9d737 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -31,6 +31,7 @@\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] [--pathspec-from-file [--pathspec-file-nul]] []\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n@@ -284,8 +285,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-\tconst char *rev;\n+\tint patch_mode = 0, pathspec_file_nul = 0, unborn;\n+\tconst char *rev, *pathspec_from_file = NULL;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n \tint intent_to_add = 0;\n@@ -306,6 +307,8 @@ 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_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n \t\tOPT_END()\n \t};\n \n@@ -316,6 +319,20 @@ 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 (pathspec_from_file) {\n+\t\tif (patch_mode)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with --patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n+\n+\t\tparse_pathspec_file(&pathspec, 0,\n+\t\t\t\t PATHSPEC_PREFER_FULL,\n+\t\t\t\t prefix, pathspec_from_file, pathspec_file_nul);\n+\t} else if (pathspec_file_nul) {\n+\t\tdie(_(\"--pathspec-file-nul requires --pathspec-from-file\"));\n+\t}\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 */\ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nnew file mode 100755\nindex 0000000000..987cc77b7d\n--- /dev/null\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -0,0 +1,155 @@\n+#!/bin/sh\n+\n+test_description='reset --pathspec-from-file'\n+\n+. ./test-lib.sh\n+\n+test_tick\n+\n+test_expect_success setup '\n+\techo A >fileA.t &&\n+\techo B >fileB.t &&\n+\techo C >fileC.t &&\n+\techo D >fileD.t &&\n+\tgit add . &&\n+\tgit commit --include . -m \"Commit\" &&\n+\tgit tag checkpoint\n+'\n+\n+restore_checkpoint () {\n+\tgit reset --hard checkpoint\n+}\n+\n+verify_expect () {\n+\tgit status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success '--pathspec-from-file from stdin' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success '--pathspec-from-file from file' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t >list &&\n+\tgit reset --pathspec-from-file=list &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\0fileB.t\\0\" | git reset --pathspec-from-file=- --pathspec-file-nul &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'quotes' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n+\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n+\t\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\ttest_must_fail verify_expect\n+'\n+\n+test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t >list &&\n+\ttest_must_fail git reset --soft --pathspec-from-file=list &&\n+\ttest_must_fail git reset --hard --pathspec-from-file=list\n+'\n+\n+test_expect_success 'only touches what was listed' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tD fileA.t\n+\t D fileB.t\n+\t D fileC.t\n+\tD fileD.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"385604","messageId":"ccda7c00-dbce-505e-7971-cbca11e9bf57@syntevo.com","threadId":"52185","inReplyTo":"e34eba95-f413-ddd3-08c6-4f5b2bed3761@gmail.com","subject":"Re: [PATCH 1/5] pathspec: add new function to parse file","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-06T15:56:10Z","receivedAt":"2019-11-06T15:56:18Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"I think I have implemented all suggestions in PatchV2. Thanks!\n"},{"id":"385605","messageId":"ffd13b85-e82d-3b7a-da0b-d28e8f546796@syntevo.com","threadId":"52185","inReplyTo":"b93592df-561f-36d4-09ce-e02d570c60da@gmail.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-06T15:56:21Z","receivedAt":"2019-11-06T15:56:28Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"I think I have implemented most suggestions in PatchV2. Thanks!\n\nOn 05.11.2019 16:03, Phillip Wood wrote:\n> I'm confused reset does not use stdin does it?\n\nI have improved commit message to clarify.\n\n> --pathspec-file would be shorter and still conveys the intent of the \n> option.\n\nI decided to stay with writing suggested by Junio.\n\n> Is this line missing a leading space?\n\nI think it's good. Maybe you were confused by patch formatting?\n\n> Alignment again\n\nI think it's good again :)\n\n> If I've understood correctly this doesn't test if a path is correctly \n> unquoted, only that it is accepted.\n\nAlready agreed that test is good, just a bit non-obvious.\n\n> s/^/--pathspec-from-file / ?\n\nI decided not to repeat '--pathspec-from-file' in every test, because \ntest package already has it in description. Is that good?\n"},{"id":"385606","messageId":"25aaaca1-1c88-d2c6-b502-cd35752ce745@syntevo.com","threadId":"52185","inReplyTo":"9ca7fa57-c438-7243-6ab1-956d8f132d37@gmail.com","subject":"Re: [PATCH 5/5] commit: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-06T15:56:29Z","receivedAt":"2019-11-06T15:56:33Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"I think I have implemented most suggestions in PatchV2. Thanks!\n\n> It might be worth tailoring the message to the command rather than \n> having exactly the same message for commit and reset\n\nI decided to move the general comment to base commit where options are \nintroduced and not repeat it where options are supported.\n\n> I wonder if there is a way of calling parse_pathspec_file() from \n> parse_and_validate_options() instead. Otherwise we end up validating \n> options here instead which is a bit messy.\n\nThe code looks a bit too entangled to support that without making it \nworse. The biggest concern I have is that parse_and_validate_options() \nwill populate `pathspec` and some other function will need to remember \nto clean it up. I like it better when `pathspec` is handled in one place.\n\nI agree that things are not perfect, but this seems to be a consequence \nof other existing problems. For example, I would have expected a \nstructure instead of a handful of global variables. That would have \nsolved many problems. However, I didn't have the bravery to dive into \nthis refactoring.\n"},{"id":"385607","messageId":"b7acd8d1-d17a-29bd-44d3-59d425842c10@syntevo.com","threadId":"52185","inReplyTo":"xmqqh83hveiz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/5] doc: reset: unify description","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-06T15:56:34Z","receivedAt":"2019-11-06T15:56:41Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"I think I have implemented most suggestions in PatchV2. Thanks!\n\n> Any time I see \"... X. This means Y.\" either in the doc or in the\n> proposed log message, I wish the author (not you in this case,\n> obviously) thought twice about rewriting so that they do not say one\n> thing and immediately have to rephrase it, i.e. either just say Y\n> without saying X, or saying X more clearly without having to say Y.\n> \n> In this case, however, I think X and Y are related but both relevant.\n> The subcommand resets the index entries for chosen paths to match\n> what is in the tree-ish, which is the same as restoring from a tree\n> to the index.\n> \n> It is not quite the opposite of adding to the index from the working\n> tree. In this sequence:\n> \n> \t$ edit newfile\n> \t$ git add newfile\n> \n> and then further\n> \n> \t$ edit newfile\n> \t$ git add newfile\n> \t$ git reset -- newfile\n> \n> we are taken back to the state _before_ any of the changes made to\n> newfile (in fact, since HEAD does not have newfile, the resulting\n> index would not know about it, either).\n\nI am a bit confused, is it correct that you don't expect me to change my \npatches?\n"},{"id":"385608","messageId":"df4cdf1d-1775-9d25-f32b-1abd57dffc31@syntevo.com","threadId":"52185","inReplyTo":"xmqqd0e5vcq9.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 3/5] reset: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-06T15:56:39Z","receivedAt":"2019-11-06T15:56:46Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"I think I have implemented most suggestions in PatchV2. Thanks!\n\n>> 2) Some UI's have started and grown with porcelain commands. Replacing\n>> existing logic with plumbing commands could be cumbersome and prone\n>> to various new problems.\n> \n> There is not a lot of sympathy for such argument ;-)\n\nLet's think that this merely reinforces the first point :)\n\n> I do not understand this part. Wouldn't it natural to expect that\n> \"-z\", when used with \"--pathspec-from-file\", tell us that the named\n> file is NUL delimited collection of records? What's different about\n> \"--pathspec-file-null\" that it is confusing to all it \"-z\"?\n> \n> I actually do not mind not having \"-z\" and using only\n> \"--pathspec-file-null\". A new option with long name that feels\n> similar to existing \"-z\" but does sufficiently different things so\n> that it needs a different name, however, feels counter-productive\n> for the purpose of using it in the UI that produces commands to be\n> learned by end-users.\n\nI didn't want to run into a situation where '-z' suddenly applies to \nmultiple things (for example, if later it's decided to allow --patch \nwith --pathspec-from-file).\n\nRegarding UI, I think that (insert our UI name here) will not use \n'--pathspec-from-file' unless there are really many files. When it does \nhappen, user will probably have an easier time understanding \n'--pathspec-file-nul' then '-z'.\n\n> When I invented the terms \"pathspec\", \"refspec\", etc. for this\n> project, I meant them to be collective nouns. A pathspec is a set\n> of pathspec elements, each of which is usually given as a separate\n> argument on the command line. So \"Read pathspec from file\" is good\n> (not \"Read pathspecs from file\").\n> \n> And \"Pathspec elements\" are separated by LF or CR/LF.\n\nI changed that. However, `git grep pathspecs` is quite interesting. My \nown taste says that \"pathspecs\" is more expected then \"pathspec\".\n\n> A tangent. Since we do not allow NUL in a pathspec element, we do\n> not even need the \"-z\" option. When we read pathspec from a file,\n> we can take any of CRLF, LF or NUL as the separator.\n> \n> Ah, sorry, that would not help very much. With \"-z\" we are allowing\n> to express pathspec elements inside which there are embedded LF or\n> CR/LF. Sorry about the noise.\n\nYes, NUL delimiter allows to avoid escaping.\n\n> - Quote end-of-here-document (EOF in the above) if you do not rely\n> on parameter interpolation inside the here document; this helps\n> readers by telling them that what is presented is used verbatim.\n\nTests overwhelmingly prefer escaped form (\\EOF) instead, so I used that.\n\n> This feeds \"fileA.tfileB.t\" without after \"fileB.t\" to\n> the command. Intended?\n\nKind of intended. I changed tests to always use trailing delimiters, and \nalso added a new dedicated test without trailing delimiter.\n"},{"id":"385609","messageId":"6cd64094-d229-ef96-86fd-e64accd759d4@syntevo.com","threadId":"52185","inReplyTo":"xmqq8sotvcar.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 4/5] doc: commit: unify description","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-06T15:56:54Z","receivedAt":"2019-11-06T15:56:57Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"I think I have implemented most suggestions in PatchV2. Thanks!\n\n> I am not sure if we want to repeat this all over the place.\n> \n> We do not say \"For details about the syntax, see the\n> 'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]\" for\n> every command that takes from the command line.\n> \n> Is your urge to suggest adding this sentence coming from that you\n> are much more familiar with than ? In other\n> words, if you were more familiar with Git, would you still be adding\n> this (and not corresponding one for )?\n\nCommit is a well known term, dating from ancient times like CVS or even \nolder.\n\nPathspec, however, is something new. When I pretend to be someone new to \ngit, I see it this way:\n1) Let's read \"git commit\" documentation\n2) Where on this commandline do I put my filename?!\n\nSo yes, I would repeat it in every location that could be popular for \nnew users.\n"},{"id":"385660","messageId":"xmqqo8xoqlvf.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"b7acd8d1-d17a-29bd-44d3-59d425842c10@syntevo.com","subject":"Re: [PATCH 2/5] doc: reset: unify description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-07T05:46:44Z","receivedAt":"2019-11-07T05:46:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexandr Miloslavskiy writes:\n\n>> It is not quite the opposite of adding to the index from the working\n>> tree. In this sequence:\n>>\n>> \t$ edit newfile\n>> \t$ git add newfile\n>>\n>> and then further\n>>\n>> \t$ edit newfile\n>> \t$ git add newfile\n>> \t$ git reset -- newfile\n>>\n>> we are taken back to the state _before_ any of the changes made to\n>> newfile (in fact, since HEAD does not have newfile, the resulting\n>> index would not know about it, either).\n>\n> I am a bit confused, is it correct that you don't expect me to change\n> my patches?\n\nI do not know if removal of the only-half-correct \"This is the\nopposite of add\" should be part of this change, or it should be a\nseparate fix. The half-wrong sentene was not introduced by this\npatch, so leaving it as-is is OK. It just leaves another thing for\nus to think about later.\n\nThanks.\n"},{"id":"385661","messageId":"xmqqk18cqlij.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"6cd64094-d229-ef96-86fd-e64accd759d4@syntevo.com","subject":"Re: [PATCH 4/5] doc: commit: unify description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-07T05:54:28Z","receivedAt":"2019-11-07T05:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexandr Miloslavskiy writes:\n\n> I think I have implemented most suggestions in PatchV2. Thanks!\n>\n>> I am not sure if we want to repeat this all over the place.\n>>\n>> We do not say \"For details about the syntax, see the\n>> 'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]\" for\n>> every command that takes from the command line.\n>>\n>> Is your urge to suggest adding this sentence coming from that you\n>> are much more familiar with than ? In other\n>> words, if you were more familiar with Git, would you still be adding\n>> this (and not corresponding one for )?\n>\n> Commit is a well known term, dating from ancient times like CVS or\n> even older.\n\nThat's more or less irrelevant. \n\nI am reacting to this from your change that you omitted quoting in\nyour reply:\n\n> +For more details about the syntax, see the 'pathspec' entry\n> +in linkgit:gitglossary[7].\n\nThat sentence is for those who have some notion of but\ndoes not know enough about its syntax.\n\nCVS does not let you specify like \"master^{/^fix frotz}~4\";\nA user a user who is familiar with CVS's commits would still want to\nrefer to the section \"For details about the syntax\".\n\nI am not advocating to add the reference to SPECIFYING REVISIONS\nsection; instead we should let readers know that every time they see\n, they can refer to the glossary for more details.\n\n> Pathspec, however, is something new.\n\nCompared to CVS, everything in Git may be new, but that was a news\nin 2010, not this year.\n"},{"id":"385696","messageId":"5121d118-6a4c-80c1-ccd5-60c3c6b5f6ee@syntevo.com","threadId":"52185","inReplyTo":"xmqqo8xoqlvf.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/5] doc: reset: unify description","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-07T11:05:10Z","receivedAt":"2019-11-07T11:05:15Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 07.11.2019 6:46, Junio C Hamano wrote:\n> I do not know if removal of the only-half-correct \"This is the\n> opposite of add\" should be part of this change, or it should be a\n> separate fix. The half-wrong sentene was not introduced by this\n> patch, so leaving it as-is is OK. It just leaves another thing for\n> us to think about later.\n\nMy feeling is that this problem is separate from the topic I'm working \non. I only touched docs to synchronize , so that I can \ncopy&paste new option description in next commit without tailoring it to \nlocal speech. Also, I must admit that upon reading your explanation, I \nfelt that I lack the experience to update the writing properly.\n"},{"id":"385699","messageId":"ffcc5857-a6c8-1cf5-f5b6-334e778c576c@syntevo.com","threadId":"52185","inReplyTo":"xmqqk18cqlij.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 4/5] doc: commit: unify description","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-07T11:39:55Z","receivedAt":"2019-11-07T11:40:04Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 07.11.2019 6:54, Junio C Hamano wrote:\n> I am reacting to this from your change that you omitted quoting in\n> your reply:\n> \n>> +For more details about the syntax, see the 'pathspec' entry\n>> +in linkgit:gitglossary[7].\n> \n> That sentence is for those who have some notion of but\n> does not know enough about its syntax.\n\nIn the perfect world, I would expect _every_ 'pathspec' word in text to \nbe an HTML-style link to a dedicated article, not just a paragraph in \nglossary.\n\nMSDN is in general a good example [1]: there, it's easy to read only a \nsmall portion of article, ignoring anything you're not interested in, \nand still have all links at hand.\n\nRegarding dedicated page: the content of 'pathspec' in glossary is \nalready long enough for a page, and it could benefit from additional \nexamples. Also, having a dedicated page makes linking easier, so that \nuser doesn't have to scroll glossary.\n\nRegarding links: I don't really understand what git's doc format allows. \nIs it just pure text in worst (or even average) case?\n\nIf it's usually something with clickable links, it could be worth to \njust insert links everywhere.\n\nIf it's usually plaintext, then \"see the 'pathspec' entry in \nlinkgit:gitglossary[7]\" is a bit too verbose to repeat on every \noccasion. Still, I'd like to see links everywhere. One big reason is to \nlet reader know that the explanation actually exists!\n\nA compromise solution is to give every article header like this:\n\n This article uses the following terms which are explained\n in linkgit:gitglossary[7]:\n * pathspec\n * tree-ish\n * refspec\n\nIf it's close to top of article, then chances are that everyone will \nnotice it. Also, it will not require extra verbosity in plaintext.\n\n> CVS does not let you specify like \"master^{/^fix frotz}~4\";\n> A user a user who is familiar with CVS's commits would still want to\n> refer to the section \"For details about the syntax\".\n> \n> I am not advocating to add the reference to SPECIFYING REVISIONS\n> section; instead we should let readers know that every time they see\n> , they can refer to the glossary for more details.\n\nI now think that my arguments apply to AND in the \nsame way. Indeed a user can't know complex variants until \nhe/she reads it in git docs.\n\n----\n\n[1] \nhttps://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea\n"},{"id":"385749","messageId":"xmqqtv7fnk5r.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"5121d118-6a4c-80c1-ccd5-60c3c6b5f6ee@syntevo.com","subject":"Re: [PATCH 2/5] doc: reset: unify description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-08T03:04:16Z","receivedAt":"2019-11-08T03:04:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexandr Miloslavskiy writes:\n\n> On 07.11.2019 6:46, Junio C Hamano wrote:\n>> I do not know if removal of the only-half-correct \"This is the\n>> opposite of add\" should be part of this change, or it should be a\n>> separate fix. The half-wrong sentene was not introduced by this\n>> patch, so leaving it as-is is OK. It just leaves another thing for\n>> us to think about later.\n>\n> My feeling is that this problem is separate from the topic I'm working\n> on. I only touched docs to synchronize , so that I can\n> copy&paste new option description in next commit without tailoring it\n> to local speech. Also, I must admit that upon reading your\n> explanation, I felt that I lack the experience to update the writing\n> properly.\n\nYeah, I do think it is cleaner to leave it as a separate issue.\n\nThanks.\n\n"},{"id":"386506","messageId":"xmqqzhgsxv77.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"96697ba0724caee556f21ada3959c4190d80c0b9.1573055478.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 2/6] pathspec: add new function to parse file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T05:59:40Z","receivedAt":"2019-11-19T05:59:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> +void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,\n> +\t\t\t unsigned flags, const char *prefix,\n> +\t\t\t const char *file, int nul_term_line)\n> +{\n> +\tstruct argv_array parsed_file = ARGV_ARRAY_INIT;\n> +\tstrbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :\n> +\t\t\t\t\t\t strbuf_getline;\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tstruct strbuf unquoted = STRBUF_INIT;\n> +\tFILE *in = NULL;\n\nUseless initialization to NULL can be dropped here.\n\n> +\tif (!strcmp(file, \"-\"))\n> +\t\tin = stdin;\n> +\telse\n> +\t\tin = xfopen(file, \"r\");\n"},{"id":"386507","messageId":"xmqqv9rgxuy1.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"f961a5155a5ec1590652ce7b5a549eead71930e1.1573055478.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/6] doc: reset: unify description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T06:05:10Z","receivedAt":"2019-11-19T06:05:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> -'git reset' [-q] [] [--] ...\n> -'git reset' (--patch | -p) [] [--] [...]\n> +'git reset' [-q] [] [--] ...\n> +'git reset' (--patch | -p) [] [--] [...]\n\nGood.\n\n> @@ -18,24 +18,26 @@ In the first and second form, copy entries from `` to the index.\n> In the third form, set the current branch head (`HEAD`) to ``,\n> optionally modifying index and working tree to match.\n> The ``/`` defaults to `HEAD` in all forms.\n> +The is used to limit the paths affected by the operation\n> +(see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n\nGood.\n\n> -'git reset' [-q] [] [--] ...::\n> -\tThis form resets the index entries for all `` to their\n> +'git reset' [-q] [] [--] ...::\n> +\tThis form resets the index entries for all `` to their\n\nThis is not so good. The original pretended as if are the\nexact pathnames, so it was sort-of OK for it to say \"for all paths\".\nSince we are highlighting the fact that these are not pathnames but\nthe patterns to match pathnames, however, the description needs a\nslight update to match, perhaps like\n\n\tths form resets the index entries for all paths that match\n\tthe `` to their...\n\n> \tstate at ``. (It does not affect the working tree or\n> \tthe current branch.)\n\nThanks.\n"},{"id":"386508","messageId":"xmqqo8x8xup8.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"cb5fc9b14dbe81703a7fc61b56f5c68c4a49ebc2.1573055478.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 6/6] commit: support the --pathspec-from-file option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T06:10:27Z","receivedAt":"2019-11-19T06:10:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> @@ -277,6 +278,18 @@ FROM UPSTREAM REBASE\" section in linkgit:git-rebase[1].)\n> \tthe last commit without committing changes that have\n> \talready been staged. If used together with `--allow-empty`\n> \tpaths are also not required, and an empty commit will be created.\n> +\t\n> +--pathspec-from-file=::\n> +\tPathspec is passed in `` instead of commandline args. If\n> +\t`` is exactly `-` then standard input is used. Pathspec\n> +\telements are separated by LF or CR/LF. Pathspec elements can be\n> +\tquoted as explained for the configuration variable `core.quotePath`\n> +\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n> +\tglobal `--literal-pathspecs`.\n\nOK.\n\n> +--pathspec-file-nul::\n> +\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n> +\tseparated with NUL character and are not expected to be quoted.\n\nAlthough it is not incorrect, \"are not expected to be quoted\" feels\na bit weak as the technical description. Are they not expected to\nbe quoted, but the command gracefully works on them even when they\nare found to be quoted?\n\nRephrasing to avoid such misinterpretation may be worth doing,\nperhaps\n\n\t... and are taken as-is without unquoting (i.e. as if\n\t`core.quotePath` is set to `false`).\n\nor something like that?\n\nThanks.\n"},{"id":"386509","messageId":"xmqqk17wxuev.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"20c4495fd31110c56cca84385bee6d1ab4a6f33e.1573055478.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 5/6] doc: commit: unify description","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-19T06:16:40Z","receivedAt":"2019-11-19T06:16:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n> +...::\n> +\tWhen pathspec is given on the command line, commit the contents of\n> +\tthe files that match the pathspec without recording the changes\n> +\talready added to the index. The contents of these files are also\n> +\tstaged for the next commit on top of what have been staged before.\n> ++\n> +For more details about the syntax, see the 'pathspec' entry\n> +in linkgit:gitglossary[7].\n\nWhat you added in [PATCH 3/6] (git reset doc) sends a slightly\ndifferent message, i.e.\n\n The is used to limit the paths affected by the operation\n (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n\nand I think that was more appropriate than what we see here. You\nare referring your readers to the glossary entry not just for the\nsyntax but also the entire concept of .\n\nIOW, it would be needed to drop \"syntax\" from \"about the \nsyntax\" from here to match the update to \"git reset\" documentation.\n\nThanks.\n"},{"id":"386524","messageId":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v2.git.1573055478.gitgitgadget@gmail.com","subject":"[PATCH v3 0/6] Add --pathspec-from-file option for reset, commit","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-19T16:48:49Z","receivedAt":"2019-11-19T16:49:01Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Changes from V2:\n\n1) Some polishing in documentation 2) = NULL removed from \nparse_pathspec_file()\n\nAlexandr Miloslavskiy (6):\n parse-options.h: add new options `--pathspec-from-file`,\n `--pathspec-file-nul`\n pathspec: add new function to parse file\n doc: reset: synchronize description\n reset: support the `--pathspec-from-file` option\n doc: commit: synchronize description\n commit: support the --pathspec-from-file option\n\n Documentation/git-commit.txt | 29 ++++--\n Documentation/git-reset.txt | 48 +++++++---\n builtin/commit.c | 25 +++++-\n builtin/reset.c | 25 +++++-\n parse-options.h | 2 +\n pathspec.c | 38 ++++++++\n pathspec.h | 10 +++\n t/t7107-reset-pathspec-file.sh | 155 ++++++++++++++++++++++++++++++++\n t/t7526-commit-pathspec-file.sh | 130 +++++++++++++++++++++++++++\n 9 files changed, 434 insertions(+), 28 deletions(-)\n create mode 100755 t/t7107-reset-pathspec-file.sh\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/445\n\nRange-diff vs v2:\n\n 1: 2dfaccf0d5 = 1: 19b80326ea parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`\n 2: 96697ba072 ! 2: 55a7c6ec3c pathspec: add new function to parse file\n @@ -45,7 +45,7 @@\n +\t\t\t\t\t\t strbuf_getline;\n +\tstruct strbuf buf = STRBUF_INIT;\n +\tstruct strbuf unquoted = STRBUF_INIT;\n -+\tFILE *in = NULL;\n ++\tFILE *in;\n +\n +\tif (!strcmp(file, \"-\"))\n +\t\tin = stdin;\n 3: f961a5155a ! 3: d9f32e523c doc: reset: unify description\n @@ -1,8 +1,8 @@\n Author: Alexandr Miloslavskiy \n \n - doc: reset: unify description\n + doc: reset: synchronize description\n \n - Synchronize it to `git add`, which has a pretty good description.\n + `git add` shows an example of good writing, follow it.\n \n Signed-off-by: Alexandr Miloslavskiy \n \n @@ -21,18 +21,17 @@\n \n DESCRIPTION\n @@\n - In the third form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n -+The is used to limit the paths affected by the operation\n -+(see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n \n -'git reset' [-q] [] [--] ...::\n -\tThis form resets the index entries for all `` to their\n +-\tstate at ``. (It does not affect the working tree or\n +-\tthe current branch.)\n +'git reset' [-q] [] [--] ...::\n -+\tThis form resets the index entries for all `` to their\n - \tstate at ``. (It does not affect the working tree or\n - \tthe current branch.)\n ++\tThis form resets the index entries for all paths that match the\n ++\t`` to their state at ``. (It does not affect\n ++\tthe working tree or the current branch.)\n +\n -This means that `git reset ` is the opposite of `git add\n -`. This command is equivalent to\n @@ -54,6 +53,20 @@\n \tInteractively select hunks in the difference between the index\n \tand `` (defaults to `HEAD`). The chosen hunks are applied\n \tin reverse to the index.\n +@@\n + \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n + \toverride the default behavior.\n + \n ++\\--::\n ++\tDo not interpret any more arguments as options.\n ++\n ++...::\n ++\tLimits the paths affected by the operation.\n +++\n ++For more details, see the 'pathspec' entry in linkgit:gitglossary[7].\n + \n + EXAMPLES\n + --------\n \n diff --git a/builtin/reset.c b/builtin/reset.c\n --- a/builtin/reset.c\n 4: d72d4f16b5 ! 4: 8a10ff881b reset: support the `--pathspec-from-file` option\n @@ -31,15 +31,13 @@\n +In the last form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n - The is used to limit the paths affected by the operation\n - (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n \n 'git reset' [-q] [] [--] ...::\n --\tThis form resets the index entries for all `` to their\n +-\tThis form resets the index entries for all paths that match the\n +'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []::\n -+\tThese forms reset the index entries matching pathspec to their\n - \tstate at ``. (It does not affect the working tree or\n - \tthe current branch.)\n ++\tThese forms reset the index entries for all paths that match the\n + \t`` to their state at ``. (It does not affect\n + \tthe working tree or the current branch.)\n +\n @@\n \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n @@ -55,10 +53,12 @@\n +\n +--pathspec-file-nul::\n +\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n -+\tseparated with NUL character and are not expected to be quoted.\n ++\tseparated with NUL character and all other characters are taken\n ++\tliterally (including newlines and quotes).\n ++\n + \\--::\n + \tDo not interpret any more arguments as options.\n \n - EXAMPLES\n - --------\n \n diff --git a/builtin/reset.c b/builtin/reset.c\n --- a/builtin/reset.c\n @@ -241,7 +241,7 @@\n +\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n +\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n +\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n -+\t\n ++\n +\tcat >expect <<-\\EOF &&\n +\t D fileA.t\n +\tEOF\n 5: 20c4495fd3 ! 5: 0b79797e77 doc: commit: unify description\n @@ -1,8 +1,8 @@\n Author: Alexandr Miloslavskiy \n \n - doc: commit: unify description\n + doc: commit: synchronize description\n \n - Synchronize it to `git add`, which has a pretty good description.\n + `git add` shows an example of good writing, follow it.\n This also better disambiguates ... header.\n \n Signed-off-by: Alexandr Miloslavskiy \n @@ -35,8 +35,7 @@\n +\talready added to the index. The contents of these files are also\n +\tstaged for the next commit on top of what have been staged before.\n ++\n -+For more details about the syntax, see the 'pathspec' entry\n -+in linkgit:gitglossary[7].\n ++For more details, see the 'pathspec' entry in linkgit:gitglossary[7].\n \n :git-commit: 1\n include::date-formats.txt[]\n 6: cb5fc9b14d ! 6: 7e48212002 commit: support the --pathspec-from-file option\n @@ -25,10 +25,9 @@\n DESCRIPTION\n -----------\n @@\n - \tthe last commit without committing changes that have\n \talready been staged. If used together with `--allow-empty`\n \tpaths are also not required, and an empty commit will be created.\n -+\t\n + \n +--pathspec-from-file=::\n +\tPathspec is passed in `` instead of commandline args. If\n +\t`` is exactly `-` then standard input is used. Pathspec\n @@ -39,10 +38,12 @@\n +\n +--pathspec-file-nul::\n +\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n -+\tseparated with NUL character and are not expected to be quoted.\n - \n ++\tseparated with NUL character and all other characters are taken\n ++\tliterally (including newlines and quotes).\n ++\n -u[]::\n --untracked-files[=]::\n + \tShow untracked files.\n \n diff --git a/builtin/commit.c b/builtin/commit.c\n --- a/builtin/commit.c\n @@ -118,7 +119,7 @@\n +test_expect_success setup '\n +\ttest_commit file0 &&\n +\tgit tag checkpoint &&\n -+\t\n ++\n +\techo A >fileA.t &&\n +\techo B >fileB.t &&\n +\techo C >fileC.t &&\n\n-- \ngitgitgadget\n"},{"id":"386527","messageId":"d9f32e523caafa0ab6f9536d764d221fb090015c.1574182135.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"[PATCH v3 3/6] doc: reset: synchronize description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-19T16:48:52Z","receivedAt":"2019-11-19T16:49:02Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`git add` shows an example of good writing, follow it.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-reset.txt | 29 ++++++++++++++++++-----------\n builtin/reset.c | 4 ++--\n 2 files changed, 20 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 97e0544d9e..d517a43e73 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state\n SYNOPSIS\n --------\n [verse]\n-'git reset' [-q] [] [--] ...\n-'git reset' (--patch | -p) [] [--] [...]\n+'git reset' [-q] [] [--] ...\n+'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n@@ -19,23 +19,23 @@ In the third form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n \n-'git reset' [-q] [] [--] ...::\n-\tThis form resets the index entries for all `` to their\n-\tstate at ``. (It does not affect the working tree or\n-\tthe current branch.)\n+'git reset' [-q] [] [--] ...::\n+\tThis form resets the index entries for all paths that match the\n+\t`` to their state at ``. (It does not affect\n+\tthe working tree or the current branch.)\n +\n-This means that `git reset ` is the opposite of `git add\n-`. This command is equivalent to\n-`git restore [--source=] --staged ...`.\n+This means that `git reset ` is the opposite of `git add\n+`. This command is equivalent to\n+`git restore [--source=] --staged ...`.\n +\n-After running `git reset ` to update the index entry, you can\n+After running `git reset ` to update the index entry, you can\n use linkgit:git-restore[1] to check the contents out of the index to\n the working tree. Alternatively, using linkgit:git-restore[1]\n and specifying a commit with `--source`, you\n can copy the contents of a path out of a commit to the index and to the\n working tree in one go.\n \n-'git reset' (--patch | -p) [] [--] [...]::\n+'git reset' (--patch | -p) [] [--] [...]::\n \tInteractively select hunks in the difference between the index\n \tand `` (defaults to `HEAD`). The chosen hunks are applied\n \tin reverse to the index.\n@@ -101,6 +101,13 @@ OPTIONS\n \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n \toverride the default behavior.\n \n+\\--::\n+\tDo not interpret any more arguments as options.\n+\n+...::\n+\tLimits the paths affected by the operation.\n++\n+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].\n \n EXAMPLES\n --------\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fdd572168b..9291c0fd72 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -30,8 +30,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] [] [--] ...\"),\n-\tN_(\"git reset --patch [] [--] [...]\"),\n+\tN_(\"git reset [-q] [] [--] ...\"),\n+\tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n \n-- \ngitgitgadget\n\n"},{"id":"386525","messageId":"55a7c6ec3cebd46b8650ad8116923676fa0e2b3a.1574182135.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"[PATCH v3 2/6] pathspec: add new function to parse file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-19T16:48:51Z","receivedAt":"2019-11-19T16:49:03Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis will be used to support the new option '--pathspec-from-file' in\n`git add`, `git-commit`, `git reset` etc.\n\nNote also that we specifically handle CR/LF line endings to support\nWindows better.\n\nTo simplify code, file is first parsed into `argv_array`. This allows\nto avoid refactoring `parse_pathspec()`.\n\nI considered adding `nul_term_line` to `flags` instead, but decided\nthat it doesn't fit there.\n\nThe new code is mostly taken from `cmd_update_index()` and\n`split_mail_conv()`.\n\nCo-authored-by: Johannes Schindelin \nSigned-off-by: Alexandr Miloslavskiy \n---\n pathspec.c | 38 ++++++++++++++++++++++++++++++++++++++\n pathspec.h | 10 ++++++++++\n 2 files changed, 48 insertions(+)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 12c2b322b3..128f27fcb7 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -3,6 +3,8 @@\n #include \"dir.h\"\n #include \"pathspec.h\"\n #include \"attr.h\"\n+#include \"argv-array.h\"\n+#include \"quote.h\"\n \n /*\n * Finds which of the given pathspecs match items in the index.\n@@ -613,6 +615,42 @@ void parse_pathspec(struct pathspec *pathspec,\n \t}\n }\n \n+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,\n+\t\t\t unsigned flags, const char *prefix,\n+\t\t\t const char *file, int nul_term_line)\n+{\n+\tstruct argv_array parsed_file = ARGV_ARRAY_INIT;\n+\tstrbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :\n+\t\t\t\t\t\t strbuf_getline;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf unquoted = STRBUF_INIT;\n+\tFILE *in;\n+\n+\tif (!strcmp(file, \"-\"))\n+\t\tin = stdin;\n+\telse\n+\t\tin = xfopen(file, \"r\");\n+\n+\twhile (getline_fn(&buf, in) != EOF) {\n+\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&unquoted);\n+\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n+\t\t\t\tdie(_(\"line is badly quoted: %s\"), buf.buf);\n+\t\t\tstrbuf_swap(&buf, &unquoted);\n+\t\t}\n+\t\targv_array_push(&parsed_file, buf.buf);\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\n+\tstrbuf_release(&unquoted);\n+\tstrbuf_release(&buf);\n+\tif (in != stdin)\n+\t\tfclose(in);\n+\n+\tparse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);\n+\targv_array_clear(&parsed_file);\n+}\n+\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src)\n {\n \tint i, j;\ndiff --git a/pathspec.h b/pathspec.h\nindex 1c18a2c90c..a27dc81ba2 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,\n \t\t unsigned flags,\n \t\t const char *prefix,\n \t\t const char **args);\n+/*\n+ * Same as parse_pathspec() but uses file as input.\n+ * When 'file' is exactly \"-\" it uses 'stdin' instead.\n+ */\n+void parse_pathspec_file(struct pathspec *pathspec,\n+\t\t\t unsigned magic_mask,\n+\t\t\t unsigned flags,\n+\t\t\t const char *prefix,\n+\t\t\t const char *file,\n+\t\t\t int nul_term_line);\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src);\n void clear_pathspec(struct pathspec *);\n \n-- \ngitgitgadget\n\n"},{"id":"386526","messageId":"0b79797e772b25c6caa22bd9761837062e7265de.1574182135.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"[PATCH v3 5/6] doc: commit: synchronize description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-19T16:48:54Z","receivedAt":"2019-11-19T16:49:04Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`git add` shows an example of good writing, follow it.\nThis also better disambiguates ... header.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex afa7b75a23..a0c44978ee 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -345,12 +345,13 @@ changes to tracked files.\n \\--::\n \tDo not interpret any more arguments as options.\n \n-...::\n-\tWhen files are given on the command line, the command\n-\tcommits the contents of the named files, without\n-\trecording the changes already staged. The contents of\n-\tthese files are also staged for the next commit on top\n-\tof what have been staged before.\n+...::\n+\tWhen pathspec is given on the command line, commit the contents of\n+\tthe files that match the pathspec without recording the changes\n+\talready added to the index. The contents of these files are also\n+\tstaged for the next commit on top of what have been staged before.\n++\n+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].\n \n :git-commit: 1\n include::date-formats.txt[]\n-- \ngitgitgadget\n\n"},{"id":"386528","messageId":"8a10ff881b01ff22517d46a94dd53a46481af9c7.1574182135.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"[PATCH v3 4/6] reset: support the `--pathspec-from-file` option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-19T16:48:53Z","receivedAt":"2019-11-19T16:49:05Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nDecisions taken for simplicity:\n1) For now, `--pathspec-from-file` is declared incompatible with\n `--patch`, even when is not `stdin`. Such use case it not\n really expected. Also, it is harder to support in `git commit`, so\n I decided to make it incompatible in all places.\n2) It is not allowed to pass pathspec in both args and file.\n\nCo-authored-by: Johannes Schindelin \nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-reset.txt | 21 ++++-\n builtin/reset.c | 21 ++++-\n t/t7107-reset-pathspec-file.sh | 155 +++++++++++++++++++++++++++++++++\n 3 files changed, 192 insertions(+), 5 deletions(-)\n create mode 100755 t/t7107-reset-pathspec-file.sh\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex d517a43e73..932080c55d 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -9,18 +9,20 @@ SYNOPSIS\n --------\n [verse]\n 'git reset' [-q] [] [--] ...\n+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []\n 'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n -----------\n-In the first and second form, copy entries from `` to the index.\n-In the third form, set the current branch head (`HEAD`) to ``,\n+In the first three forms, copy entries from `` to the index.\n+In the last form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n \n 'git reset' [-q] [] [--] ...::\n-\tThis form resets the index entries for all paths that match the\n+'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []::\n+\tThese forms reset the index entries for all paths that match the\n \t`` to their state at ``. (It does not affect\n \tthe working tree or the current branch.)\n +\n@@ -101,6 +103,19 @@ OPTIONS\n \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n \toverride the default behavior.\n \n+--pathspec-from-file=::\n+\tPathspec is passed in `` instead of commandline args. If\n+\t`` is exactly `-` then standard input is used. Pathspec\n+\telements are separated by LF or CR/LF. Pathspec elements can be\n+\tquoted as explained for the configuration variable `core.quotePath`\n+\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n+\tglobal `--literal-pathspecs`.\n+\n+--pathspec-file-nul::\n+\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n+\tseparated with NUL character and all other characters are taken\n+\tliterally (including newlines and quotes).\n+\n \\--::\n \tDo not interpret any more arguments as options.\n \ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 9291c0fd72..246bf9d737 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -31,6 +31,7 @@\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] [--pathspec-from-file [--pathspec-file-nul]] []\"),\n \tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n@@ -284,8 +285,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-\tconst char *rev;\n+\tint patch_mode = 0, pathspec_file_nul = 0, unborn;\n+\tconst char *rev, *pathspec_from_file = NULL;\n \tstruct object_id oid;\n \tstruct pathspec pathspec;\n \tint intent_to_add = 0;\n@@ -306,6 +307,8 @@ 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_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n \t\tOPT_END()\n \t};\n \n@@ -316,6 +319,20 @@ 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 (pathspec_from_file) {\n+\t\tif (patch_mode)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with --patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n+\n+\t\tparse_pathspec_file(&pathspec, 0,\n+\t\t\t\t PATHSPEC_PREFER_FULL,\n+\t\t\t\t prefix, pathspec_from_file, pathspec_file_nul);\n+\t} else if (pathspec_file_nul) {\n+\t\tdie(_(\"--pathspec-file-nul requires --pathspec-from-file\"));\n+\t}\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 */\ndiff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh\nnew file mode 100755\nindex 0000000000..6b1a731fff\n--- /dev/null\n+++ b/t/t7107-reset-pathspec-file.sh\n@@ -0,0 +1,155 @@\n+#!/bin/sh\n+\n+test_description='reset --pathspec-from-file'\n+\n+. ./test-lib.sh\n+\n+test_tick\n+\n+test_expect_success setup '\n+\techo A >fileA.t &&\n+\techo B >fileB.t &&\n+\techo C >fileC.t &&\n+\techo D >fileD.t &&\n+\tgit add . &&\n+\tgit commit --include . -m \"Commit\" &&\n+\tgit tag checkpoint\n+'\n+\n+restore_checkpoint () {\n+\tgit reset --hard checkpoint\n+}\n+\n+verify_expect () {\n+\tgit status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success '--pathspec-from-file from stdin' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success '--pathspec-from-file from file' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t >list &&\n+\tgit reset --pathspec-from-file=list &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\0fileB.t\\0\" | git reset --pathspec-from-file=- --pathspec-file-nul &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\nfileB.t\\n\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\nfileB.t\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t &&\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\t D fileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'quotes' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\tprintf \"\\\"file\\\\101.t\\\"\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\t# Note: \"git reset\" has not yet learned to fail on wrong pathspecs\n+\tgit reset --pathspec-from-file=list --pathspec-file-nul &&\n+\n+\tcat >expect <<-\\EOF &&\n+\t D fileA.t\n+\tEOF\n+\ttest_must_fail verify_expect\n+'\n+\n+test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t &&\n+\techo fileA.t >list &&\n+\ttest_must_fail git reset --soft --pathspec-from-file=list &&\n+\ttest_must_fail git reset --hard --pathspec-from-file=list\n+'\n+\n+test_expect_success 'only touches what was listed' '\n+\trestore_checkpoint &&\n+\n+\tgit rm fileA.t fileB.t fileC.t fileD.t &&\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git reset --pathspec-from-file=- &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tD fileA.t\n+\t D fileB.t\n+\t D fileC.t\n+\tD fileD.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"386529","messageId":"19b80326ead8bdc44b5d8ca951feb3d6c1f5493c.1574182135.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"[PATCH v3 1/6] parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-19T16:48:50Z","receivedAt":"2019-11-19T16:49:06Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSupport for various porcelain commands will arrive via additional\npatches.\n\n`--pathspec-from-file` solves the problem of commandline length limit\nfor UIs built on top of git. Plumbing commands are not always a good\nfit, for two major reasons:\n1) Some UIs show executed commands to user. In this case, porcelain\n commands are expected. One reason for that is letting user learn git\n commands by clicking UI buttons. The other reason is letting user\n study the history of commands in case of any unexpected results. Both\n of these will lose most of their value if UI uses combinations of\n arcane plumbing commands.\n2) Some UIs have started and grown with porcelain commands. Replacing\n existing logic with plumbing commands could be cumbersome and prone\n to various new problems.\n\n`--pathspec-from-file` will behave very close to pathspec passed in\ncommandline args, so that switching from one to another is simple.\n\n`--pathspec-from-file` will read either a specified file or `stdin`\n(when file is exactly \"-\"). Reading from file is a good way to avoid\ncompeting for `stdin`, and also gives some extra flexibility.\n\n`--pathspec-file-nul` switch mirrors `-z` already used in various\nplaces. Some porcelain commands, such as `git commit`, already use\n`-z`, therefore it needed a new unambiguous name.\n\nNew options do not have shorthands to avoid shorthand conflicts. It is\nnot expected that they will be typed in console.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n parse-options.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/parse-options.h b/parse-options.h\nindex 38a33a087e..c6cc01e715 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -330,5 +330,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)\n #define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)\n #define OPT_CLEANUP(v) OPT_STRING(0, \"cleanup\", v, N_(\"mode\"), N_(\"how to strip spaces and #comments from message\"))\n+#define OPT_PATHSPEC_FROM_FILE(v) OPT_FILENAME(0, \"pathspec-from-file\", v, N_(\"read pathspec from file\"))\n+#define OPT_PATHSPEC_FILE_NUL(v) OPT_BOOL(0, \"pathspec-file-nul\", v, N_(\"with --pathspec-from-file, pathspec elements are separated with NUL character\"))\n \n #endif\n-- \ngitgitgadget\n\n"},{"id":"386530","messageId":"7e482120020faf403daf8fbd517fbf27cebe452c.1574182135.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"[PATCH v3 6/6] commit: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-19T16:48:55Z","receivedAt":"2019-11-19T16:49:08Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nDecisions taken for simplicity:\n1) For now, `--pathspec-from-file` is declared incompatible with\n `--interactive/--patch`, even when is not `stdin`. Such use\n case it not really expected. Also, it would require changes to\n `interactive_add()`.\n2) It is not allowed to pass pathspec in both args and file.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 16 +++-\n builtin/commit.c | 25 +++++-\n t/t7526-commit-pathspec-file.sh | 130 ++++++++++++++++++++++++++++++++\n 3 files changed, 166 insertions(+), 5 deletions(-)\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex a0c44978ee..ced5a9beab 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,8 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n+\t [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -278,6 +279,19 @@ FROM UPSTREAM REBASE\" section in linkgit:git-rebase[1].)\n \talready been staged. If used together with `--allow-empty`\n \tpaths are also not required, and an empty commit will be created.\n \n+--pathspec-from-file=::\n+\tPathspec is passed in `` instead of commandline args. If\n+\t`` is exactly `-` then standard input is used. Pathspec\n+\telements are separated by LF or CR/LF. Pathspec elements can be\n+\tquoted as explained for the configuration variable `core.quotePath`\n+\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n+\tglobal `--literal-pathspecs`.\n+\n+--pathspec-file-nul::\n+\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n+\tseparated with NUL character and all other characters are taken\n+\tliterally (including newlines and quotes).\n+\n -u[]::\n --untracked-files[=]::\n \tShow untracked files.\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e588bc6ad3..ed40729355 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n-static int no_post_rewrite, allow_empty_message;\n+static int no_post_rewrite, allow_empty_message, pathspec_file_nul;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n-static char *sign_commit;\n+static char *sign_commit, *pathspec_from_file;\n \n /*\n * The default commit message cleanup mode will remove the lines\n@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\t PATHSPEC_PREFER_FULL,\n \t\t prefix, argv);\n \n+\tif (pathspec_from_file) {\n+\t\tif (interactive)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n+\n+\t\tparse_pathspec_file(&pathspec, 0,\n+\t\t\t\t PATHSPEC_PREFER_FULL,\n+\t\t\t\t prefix, pathspec_from_file, pathspec_file_nul);\n+\t} else if (pathspec_file_nul) {\n+\t\tdie(_(\"--pathspec-file-nul requires --pathspec-from-file\"));\n+\t}\n+\n+\tif (!pathspec.nr && (also || (only && !amend && !allow_empty)))\n+\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n+\n \tif (read_cache_preload(&pathspec) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (also + only + all + interactive > 1)\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n-\tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n-\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n \tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n \n \thandle_untracked_files_arg(s);\n@@ -1535,6 +1550,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"amend\", &amend, N_(\"amend previous commit\")),\n \t\tOPT_BOOL(0, \"no-post-rewrite\", &no_post_rewrite, N_(\"bypass post-rewrite hook\")),\n \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n \t\t/* end commit contents options */\n \n \t\tOPT_HIDDEN_BOOL(0, \"allow-empty\", &allow_empty,\ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nnew file mode 100755\nindex 0000000000..a06b683534\n--- /dev/null\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -0,0 +1,130 @@\n+#!/bin/sh\n+\n+test_description='commit --pathspec-from-file'\n+\n+. ./test-lib.sh\n+\n+test_tick\n+\n+test_expect_success setup '\n+\ttest_commit file0 &&\n+\tgit tag checkpoint &&\n+\n+\techo A >fileA.t &&\n+\techo B >fileB.t &&\n+\techo C >fileC.t &&\n+\techo D >fileD.t &&\n+\tgit add fileA.t fileB.t fileC.t fileD.t\n+'\n+\n+restore_checkpoint () {\n+\tgit reset --soft checkpoint\n+}\n+\n+verify_expect () {\n+\tgit diff-tree --no-commit-id --name-status -r HEAD >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success '--pathspec-from-file from stdin' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success '--pathspec-from-file from file' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t >list &&\n+\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'quotes' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect expect\n+'\n+\n+test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n+'\n+\n+test_expect_success 'only touches what was listed' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileB.t\n+\tA\tfileC.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_done\n-- \ngitgitgadget\n"},{"id":"386531","messageId":"6b37c14b-d618-7cc9-150d-ef95f5402ab1@syntevo.com","threadId":"52185","inReplyTo":"xmqqzhgsxv77.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/6] pathspec: add new function to parse file","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-19T16:50:39Z","receivedAt":"2019-11-19T16:50:46Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 19.11.2019 6:59, Junio C Hamano wrote:\n>> +\tFILE *in = NULL;\n> \n> Useless initialization to NULL can be dropped here.\n\nChanged in V3.\n"},{"id":"386532","messageId":"df10e5bc-3e60-7fab-c66b-583a196a2423@syntevo.com","threadId":"52185","inReplyTo":"xmqqv9rgxuy1.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 3/6] doc: reset: unify description","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-19T16:52:17Z","receivedAt":"2019-11-19T16:52:22Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 19.11.2019 7:05, Junio C Hamano wrote:\n\n>> -'git reset' [-q] [] [--] ...::\n>> -\tThis form resets the index entries for all `` to their\n>> +'git reset' [-q] [] [--] ...::\n>> +\tThis form resets the index entries for all `` to their\n> \n> This is not so good. The original pretended as if are the\n> exact pathnames, so it was sort-of OK for it to say \"for all paths\".\n> Since we are highlighting the fact that these are not pathnames but\n> the patterns to match pathnames, however, the description needs a\n> slight update to match, perhaps like\n> \n> \tths form resets the index entries for all paths that match\n> \tthe `` to their...\n> \n>> \tstate at ``. (It does not affect the working tree or\n>> \tthe current branch.)\n\nYes, I wasn't paying enough attention.\nChanged in V3, thanks!\n"},{"id":"386533","messageId":"839aaf51-147f-19b2-514f-bf9b1ea78101@syntevo.com","threadId":"52185","inReplyTo":"xmqqk17wxuev.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 5/6] doc: commit: unify description","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-19T16:53:41Z","receivedAt":"2019-11-19T16:53:47Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 19.11.2019 7:16, Junio C Hamano wrote:\n>> +For more details about the syntax, see the 'pathspec' entry\n>> +in linkgit:gitglossary[7].\n> \n> What you added in [PATCH 3/6] (git reset doc) sends a slightly\n> different message, i.e.\n> \n> The is used to limit the paths affected by the operation\n> (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n> \n> and I think that was more appropriate than what we see here. You\n> are referring your readers to the glossary entry not just for the\n> syntax but also the entire concept of .\n> \n> IOW, it would be needed to drop \"syntax\" from \"about the \n> syntax\" from here to match the update to \"git reset\" documentation.\n\nI shortened it even more in V3, I think it's better now?\n"},{"id":"386534","messageId":"74889f88-b5dd-b1bd-f619-9201ce14db29@syntevo.com","threadId":"52185","inReplyTo":"xmqqo8x8xup8.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 6/6] commit: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-19T16:56:06Z","receivedAt":"2019-11-19T16:56:13Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 19.11.2019 7:10, Junio C Hamano wrote:\n>> +--pathspec-file-nul::\n>> +\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n>> +\tseparated with NUL character and are not expected to be quoted.\n> \n> Although it is not incorrect, \"are not expected to be quoted\" feels\n> a bit weak as the technical description. Are they not expected to\n> be quoted, but the command gracefully works on them even when they\n> are found to be quoted?\n> \n> Rephrasing to avoid such misinterpretation may be worth doing,\n> perhaps\n> \n> \t... and are taken as-is without unquoting (i.e. as if\n> \t`core.quotePath` is set to `false`).\n> \n> or something like that?\n\nI think that a reference to `core.quotePath` could be confusing here, \nbecause it doesn't really affect the option. I have reworded it in V3, \nplease see if you like it.\n\nThanks for pointing out!\n"},{"id":"386536","messageId":"1d3cb1f6-1537-0619-6484-9bbd187c67fe@syntevo.com","threadId":"52185","inReplyTo":"xmqqk17wxuev.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 5/6] doc: commit: unify description","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-19T17:02:29Z","receivedAt":"2019-11-19T17:02:34Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 19.11.2019 7:16, Junio C Hamano wrote:\n\n>> +For more details about the syntax, see the 'pathspec' entry\n>> +in linkgit:gitglossary[7].\n> \n> What you added in [PATCH 3/6] (git reset doc) sends a slightly\n> different message, i.e.\n> \n> The is used to limit the paths affected by the operation\n> (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).\n> \n> and I think that was more appropriate than what we see here. You\n> are referring your readers to the glossary entry not just for the\n> syntax but also the entire concept of .\n\nThis has shown me that I didn't synchronize docs enough. I have studied \ndocs for other commands and found out that most of them list \nin a separate paragraph under options.\n\nI find it very reasonable, because that's where I would normally expect \nit as a reader, together with other options.\n\nSo I adjusted 'git-reset' docs to also list under options.\n\n"},{"id":"386609","messageId":"xmqqa78rchxl.fsf@gitster-ct.c.googlers.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/6] Add --pathspec-from-file option for reset, commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-20T04:04:06Z","receivedAt":"2019-11-20T04:04:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> Changes from V2:\n>\n> 1) Some polishing in documentation 2) = NULL removed from \n> parse_pathspec_file()\n>\n> Alexandr Miloslavskiy (6):\n> parse-options.h: add new options `--pathspec-from-file`,\n> `--pathspec-file-nul`\n> pathspec: add new function to parse file\n> doc: reset: synchronize description\n> reset: support the `--pathspec-from-file` option\n> doc: commit: synchronize description\n> commit: support the --pathspec-from-file option\n\nNicely done. I think this is now ready for 'next'.\n\nThanks.\n"},{"id":"386623","messageId":"bbf2f57c-245d-e163-deee-724d524cb8c3@syntevo.com","threadId":"52185","inReplyTo":"xmqqa78rchxl.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 0/6] Add --pathspec-from-file option for reset, commit","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2019-11-20T09:22:11Z","receivedAt":"2019-11-20T09:22:16Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 20.11.2019 5:04, Junio C Hamano wrote:\n> Nicely done. I think this is now ready for 'next'.\n\nWonderful, thanks for your assistance!\n\nI will continue with patches for other commands, then.\n"},{"id":"387439","messageId":"cca5aee39229d0d54d88c7dae18dafa956473559.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 01/13] parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:06Z","receivedAt":"2019-12-03T14:02:24Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSupport for various porcelain commands will arrive via additional\npatches.\n\n`--pathspec-from-file` solves the problem of commandline length limit\nfor UIs built on top of git. Plumbing commands are not always a good\nfit, for two major reasons:\n1) Some UIs show executed commands to user. In this case, porcelain\n commands are expected. One reason for that is letting user learn git\n commands by clicking UI buttons. The other reason is letting user\n study the history of commands in case of any unexpected results. Both\n of these will lose most of their value if UI uses combinations of\n arcane plumbing commands.\n2) Some UIs have started and grown with porcelain commands. Replacing\n existing logic with plumbing commands could be cumbersome and prone\n to various new problems.\n\n`--pathspec-from-file` will behave very close to pathspec passed in\ncommandline args, so that switching from one to another is simple.\n\n`--pathspec-from-file` will read either a specified file or `stdin`\n(when file is exactly \"-\"). Reading from file is a good way to avoid\ncompeting for `stdin`, and also gives some extra flexibility.\n\n`--pathspec-file-nul` switch mirrors `-z` already used in various\nplaces. Some porcelain commands, such as `git commit`, already use\n`-z`, therefore it needed a new unambiguous name.\n\nNew options do not have shorthands to avoid shorthand conflicts. It is\nnot expected that they will be typed in console.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n parse-options.h | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/parse-options.h b/parse-options.h\nindex 38a33a087e..c6cc01e715 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -330,5 +330,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);\n #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH(\"with\", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)\n #define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH(\"without\", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)\n #define OPT_CLEANUP(v) OPT_STRING(0, \"cleanup\", v, N_(\"mode\"), N_(\"how to strip spaces and #comments from message\"))\n+#define OPT_PATHSPEC_FROM_FILE(v) OPT_FILENAME(0, \"pathspec-from-file\", v, N_(\"read pathspec from file\"))\n+#define OPT_PATHSPEC_FILE_NUL(v) OPT_BOOL(0, \"pathspec-file-nul\", v, N_(\"with --pathspec-from-file, pathspec elements are separated with NUL character\"))\n \n #endif\n-- \ngitgitgadget\n\n"},{"id":"387440","messageId":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v3.git.1574182135.gitgitgadget@gmail.com","subject":"[PATCH v4 00/13] Add --pathspec-from-file option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:05Z","receivedAt":"2019-12-03T14:02:24Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Changes from V3:\n================\n\nThe branch was rebased onto latest master. These patches remain unchanged\nsince they were accepted in V3: parse-options.h: add new options \n--pathspec-from-file, --pathspec-file-nulpathspec: add new function to parse\nfile doc: reset: synchronize description reset: support the \n--pathspec-from-file option doc: commit: synchronize description commit:\nsupport the --pathspec-from-file option These patches are new, extending\nsupport to more git commands: cmd_add: prepare for next patch add: support\nthe --pathspec-from-file option doc: checkout: remove duplicate synopsis\ndoc: checkout: fix broken text reference doc: checkout: synchronize \ndescription doc: restore: synchronize description checkout, restore: support\nthe --pathspec-from-file option\n\nCc: Phillip Wood phillip.wood123@gmail.com [phillip.wood123@gmail.com]\n\nAlexandr Miloslavskiy (13):\n parse-options.h: add new options `--pathspec-from-file`,\n `--pathspec-file-nul`\n pathspec: add new function to parse file\n doc: reset: synchronize description\n reset: support the `--pathspec-from-file` option\n doc: commit: synchronize description\n commit: support the --pathspec-from-file option\n cmd_add: prepare for next patch\n add: support the --pathspec-from-file option\n doc: checkout: remove duplicate synopsis\n doc: checkout: fix broken text reference\n doc: checkout: synchronize description\n doc: restore: synchronize description\n checkout, restore: support the --pathspec-from-file option\n\n Documentation/git-add.txt | 16 ++-\n Documentation/git-checkout.txt | 50 +++++++---\n Documentation/git-commit.txt | 29 ++++--\n Documentation/git-reset.txt | 48 ++++++---\n Documentation/git-restore.txt | 26 ++++-\n builtin/add.c | 60 ++++++++----\n builtin/checkout.c | 31 +++++-\n builtin/commit.c | 25 ++++-\n builtin/reset.c | 25 ++++-\n parse-options.h | 2 +\n pathspec.c | 38 ++++++++\n pathspec.h | 10 ++\n t/t2026-checkout-pathspec-file.sh | 139 +++++++++++++++++++++++++++\n t/t2072-restore-pathspec-file.sh | 139 +++++++++++++++++++++++++++\n t/t3704-add-pathspec-file.sh | 127 ++++++++++++++++++++++++\n t/t7107-reset-pathspec-file.sh | 155 ++++++++++++++++++++++++++++++\n t/t7526-commit-pathspec-file.sh | 130 +++++++++++++++++++++++++\n t/t9902-completion.sh | 2 +\n 18 files changed, 982 insertions(+), 70 deletions(-)\n create mode 100755 t/t2026-checkout-pathspec-file.sh\n create mode 100755 t/t2072-restore-pathspec-file.sh\n create mode 100755 t/t3704-add-pathspec-file.sh\n create mode 100755 t/t7107-reset-pathspec-file.sh\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\n\nbase-commit: 228f53135a4a41a37b6be8e4d6e2b6153db4a8ed\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/445\n\nRange-diff vs v3:\n\n 1: 19b80326ea = 1: cca5aee392 parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`\n 2: 55a7c6ec3c = 2: fea64dfbf9 pathspec: add new function to parse file\n 3: d9f32e523c = 3: 1182ba3953 doc: reset: synchronize description\n 4: 8a10ff881b = 4: cea470fc91 reset: support the `--pathspec-from-file` option\n 5: 0b79797e77 = 5: 0e1ac7e8a7 doc: commit: synchronize description\n 6: 7e48212002 = 6: c877866c13 commit: support the --pathspec-from-file option\n -: ---------- > 7: a97910cb55 cmd_add: prepare for next patch\n -: ---------- > 8: 9a62da3470 add: support the --pathspec-from-file option\n -: ---------- > 9: 5e449c8d29 doc: checkout: remove duplicate synopsis\n -: ---------- > 10: a498dda97b doc: checkout: fix broken text reference\n -: ---------- > 11: fd166755aa doc: checkout: synchronize description\n -: ---------- > 12: 9e37a740e6 doc: restore: synchronize description\n -: ---------- > 13: c4dd4eaf13 checkout, restore: support the --pathspec-from-file option\n\n-- \ngitgitgadget\n"},{"id":"387441","messageId":"1182ba39535bc7c0cc4a241bd19d4c66cf6277d7.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 03/13] doc: reset: synchronize description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:08Z","receivedAt":"2019-12-03T14:02:27Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`git add` shows an example of good writing, follow it.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-reset.txt | 29 ++++++++++++++++++-----------\n builtin/reset.c | 4 ++--\n 2 files changed, 20 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 97e0544d9e..d517a43e73 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state\n SYNOPSIS\n --------\n [verse]\n-'git reset' [-q] [] [--] ...\n-'git reset' (--patch | -p) [] [--] [...]\n+'git reset' [-q] [] [--] ...\n+'git reset' (--patch | -p) [] [--] [...]\n 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] []\n \n DESCRIPTION\n@@ -19,23 +19,23 @@ In the third form, set the current branch head (`HEAD`) to ``,\n optionally modifying index and working tree to match.\n The ``/`` defaults to `HEAD` in all forms.\n \n-'git reset' [-q] [] [--] ...::\n-\tThis form resets the index entries for all `` to their\n-\tstate at ``. (It does not affect the working tree or\n-\tthe current branch.)\n+'git reset' [-q] [] [--] ...::\n+\tThis form resets the index entries for all paths that match the\n+\t`` to their state at ``. (It does not affect\n+\tthe working tree or the current branch.)\n +\n-This means that `git reset ` is the opposite of `git add\n-`. This command is equivalent to\n-`git restore [--source=] --staged ...`.\n+This means that `git reset ` is the opposite of `git add\n+`. This command is equivalent to\n+`git restore [--source=] --staged ...`.\n +\n-After running `git reset ` to update the index entry, you can\n+After running `git reset ` to update the index entry, you can\n use linkgit:git-restore[1] to check the contents out of the index to\n the working tree. Alternatively, using linkgit:git-restore[1]\n and specifying a commit with `--source`, you\n can copy the contents of a path out of a commit to the index and to the\n working tree in one go.\n \n-'git reset' (--patch | -p) [] [--] [...]::\n+'git reset' (--patch | -p) [] [--] [...]::\n \tInteractively select hunks in the difference between the index\n \tand `` (defaults to `HEAD`). The chosen hunks are applied\n \tin reverse to the index.\n@@ -101,6 +101,13 @@ OPTIONS\n \t`reset.quiet` config option. `--quiet` and `--no-quiet` will\n \toverride the default behavior.\n \n+\\--::\n+\tDo not interpret any more arguments as options.\n+\n+...::\n+\tLimits the paths affected by the operation.\n++\n+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].\n \n EXAMPLES\n --------\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fdd572168b..9291c0fd72 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -30,8 +30,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] [] [--] ...\"),\n-\tN_(\"git reset --patch [] [--] [...]\"),\n+\tN_(\"git reset [-q] [] [--] ...\"),\n+\tN_(\"git reset --patch [] [--] [...]\"),\n \tNULL\n };\n \n-- \ngitgitgadget\n\n"},{"id":"387442","messageId":"fea64dfbf9085b45192f91aaaabd6143ae9469bc.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 02/13] pathspec: add new function to parse file","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:07Z","receivedAt":"2019-12-03T14:02:28Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis will be used to support the new option '--pathspec-from-file' in\n`git add`, `git-commit`, `git reset` etc.\n\nNote also that we specifically handle CR/LF line endings to support\nWindows better.\n\nTo simplify code, file is first parsed into `argv_array`. This allows\nto avoid refactoring `parse_pathspec()`.\n\nI considered adding `nul_term_line` to `flags` instead, but decided\nthat it doesn't fit there.\n\nThe new code is mostly taken from `cmd_update_index()` and\n`split_mail_conv()`.\n\nCo-authored-by: Johannes Schindelin \nSigned-off-by: Alexandr Miloslavskiy \n---\n pathspec.c | 38 ++++++++++++++++++++++++++++++++++++++\n pathspec.h | 10 ++++++++++\n 2 files changed, 48 insertions(+)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 12c2b322b3..128f27fcb7 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -3,6 +3,8 @@\n #include \"dir.h\"\n #include \"pathspec.h\"\n #include \"attr.h\"\n+#include \"argv-array.h\"\n+#include \"quote.h\"\n \n /*\n * Finds which of the given pathspecs match items in the index.\n@@ -613,6 +615,42 @@ void parse_pathspec(struct pathspec *pathspec,\n \t}\n }\n \n+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,\n+\t\t\t unsigned flags, const char *prefix,\n+\t\t\t const char *file, int nul_term_line)\n+{\n+\tstruct argv_array parsed_file = ARGV_ARRAY_INIT;\n+\tstrbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :\n+\t\t\t\t\t\t strbuf_getline;\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tstruct strbuf unquoted = STRBUF_INIT;\n+\tFILE *in;\n+\n+\tif (!strcmp(file, \"-\"))\n+\t\tin = stdin;\n+\telse\n+\t\tin = xfopen(file, \"r\");\n+\n+\twhile (getline_fn(&buf, in) != EOF) {\n+\t\tif (!nul_term_line && buf.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&unquoted);\n+\t\t\tif (unquote_c_style(&unquoted, buf.buf, NULL))\n+\t\t\t\tdie(_(\"line is badly quoted: %s\"), buf.buf);\n+\t\t\tstrbuf_swap(&buf, &unquoted);\n+\t\t}\n+\t\targv_array_push(&parsed_file, buf.buf);\n+\t\tstrbuf_reset(&buf);\n+\t}\n+\n+\tstrbuf_release(&unquoted);\n+\tstrbuf_release(&buf);\n+\tif (in != stdin)\n+\t\tfclose(in);\n+\n+\tparse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);\n+\targv_array_clear(&parsed_file);\n+}\n+\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src)\n {\n \tint i, j;\ndiff --git a/pathspec.h b/pathspec.h\nindex 1c18a2c90c..a27dc81ba2 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,\n \t\t unsigned flags,\n \t\t const char *prefix,\n \t\t const char **args);\n+/*\n+ * Same as parse_pathspec() but uses file as input.\n+ * When 'file' is exactly \"-\" it uses 'stdin' instead.\n+ */\n+void parse_pathspec_file(struct pathspec *pathspec,\n+\t\t\t unsigned magic_mask,\n+\t\t\t unsigned flags,\n+\t\t\t const char *prefix,\n+\t\t\t const char *file,\n+\t\t\t int nul_term_line);\n void copy_pathspec(struct pathspec *dst, const struct pathspec *src);\n void clear_pathspec(struct pathspec *);\n \n-- \ngitgitgadget\n\n"},{"id":"387443","messageId":"0e1ac7e8a70ae0d508b71bf91a0e0b8f666cbe34.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 05/13] doc: commit: synchronize description","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:10Z","receivedAt":"2019-12-03T14:02:28Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`git add` shows an example of good writing, follow it.\nThis also better disambiguates ... header.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 15 ++++++++-------\n 1 file changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex afa7b75a23..a0c44978ee 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -345,12 +345,13 @@ changes to tracked files.\n \\--::\n \tDo not interpret any more arguments as options.\n \n-...::\n-\tWhen files are given on the command line, the command\n-\tcommits the contents of the named files, without\n-\trecording the changes already staged. The contents of\n-\tthese files are also staged for the next commit on top\n-\tof what have been staged before.\n+...::\n+\tWhen pathspec is given on the command line, commit the contents of\n+\tthe files that match the pathspec without recording the changes\n+\talready added to the index. The contents of these files are also\n+\tstaged for the next commit on top of what have been staged before.\n++\n+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].\n \n :git-commit: 1\n include::date-formats.txt[]\n-- \ngitgitgadget\n\n"},{"id":"387444","messageId":"c877866c1323f4562564fa28cd355e413ca4d6b5.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 06/13] commit: support the --pathspec-from-file option","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:11Z","receivedAt":"2019-12-03T14:02:29Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nDecisions taken for simplicity:\n1) For now, `--pathspec-from-file` is declared incompatible with\n `--interactive/--patch`, even when is not `stdin`. Such use\n case it not really expected. Also, it would require changes to\n `interactive_add()`.\n2) It is not allowed to pass pathspec in both args and file.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-commit.txt | 16 +++-\n builtin/commit.c | 25 +++++-\n t/t7526-commit-pathspec-file.sh | 130 ++++++++++++++++++++++++++++++++\n 3 files changed, 166 insertions(+), 5 deletions(-)\n create mode 100755 t/t7526-commit-pathspec-file.sh\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex a0c44978ee..ced5a9beab 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -13,7 +13,8 @@ SYNOPSIS\n \t [-F | -m ] [--reset-author] [--allow-empty]\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n-\t [-i | -o] [-S[]] [--] [...]\n+\t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n+\t [-S[]] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -278,6 +279,19 @@ FROM UPSTREAM REBASE\" section in linkgit:git-rebase[1].)\n \talready been staged. If used together with `--allow-empty`\n \tpaths are also not required, and an empty commit will be created.\n \n+--pathspec-from-file=::\n+\tPathspec is passed in `` instead of commandline args. If\n+\t`` is exactly `-` then standard input is used. Pathspec\n+\telements are separated by LF or CR/LF. Pathspec elements can be\n+\tquoted as explained for the configuration variable `core.quotePath`\n+\t(see linkgit:git-config[1]). See also `--pathspec-file-nul` and\n+\tglobal `--literal-pathspecs`.\n+\n+--pathspec-file-nul::\n+\tOnly meaningful with `--pathspec-from-file`. Pathspec elements are\n+\tseparated with NUL character and all other characters are taken\n+\tliterally (including newlines and quotes).\n+\n -u[]::\n --untracked-files[=]::\n \tShow untracked files.\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 294dc574cd..2db2ad0de4 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;\n static int edit_flag = -1; /* unspecified */\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int config_commit_verbose = -1; /* unspecified */\n-static int no_post_rewrite, allow_empty_message;\n+static int no_post_rewrite, allow_empty_message, pathspec_file_nul;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n-static char *sign_commit;\n+static char *sign_commit, *pathspec_from_file;\n \n /*\n * The default commit message cleanup mode will remove the lines\n@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\t PATHSPEC_PREFER_FULL,\n \t\t prefix, argv);\n \n+\tif (pathspec_from_file) {\n+\t\tif (interactive)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n+\n+\t\tif (pathspec.nr)\n+\t\t\tdie(_(\"--pathspec-from-file is incompatible with pathspec arguments\"));\n+\n+\t\tparse_pathspec_file(&pathspec, 0,\n+\t\t\t\t PATHSPEC_PREFER_FULL,\n+\t\t\t\t prefix, pathspec_from_file, pathspec_file_nul);\n+\t} else if (pathspec_file_nul) {\n+\t\tdie(_(\"--pathspec-file-nul requires --pathspec-from-file\"));\n+\t}\n+\n+\tif (!pathspec.nr && (also || (only && !amend && !allow_empty)))\n+\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n+\n \tif (read_cache_preload(&pathspec) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (also + only + all + interactive > 1)\n \t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n-\tif (argc == 0 && (also || (only && !amend && !allow_empty)))\n-\t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n \tcleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);\n \n \thandle_untracked_files_arg(s);\n@@ -1513,6 +1528,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL(0, \"amend\", &amend, N_(\"amend previous commit\")),\n \t\tOPT_BOOL(0, \"no-post-rewrite\", &no_post_rewrite, N_(\"bypass post-rewrite hook\")),\n \t\t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, N_(\"mode\"), N_(\"show untracked files, optional modes: all, normal, no. (Default: all)\"), PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n+\t\tOPT_PATHSPEC_FROM_FILE(&pathspec_from_file),\n+\t\tOPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),\n \t\t/* end commit contents options */\n \n \t\tOPT_HIDDEN_BOOL(0, \"allow-empty\", &allow_empty,\ndiff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh\nnew file mode 100755\nindex 0000000000..a06b683534\n--- /dev/null\n+++ b/t/t7526-commit-pathspec-file.sh\n@@ -0,0 +1,130 @@\n+#!/bin/sh\n+\n+test_description='commit --pathspec-from-file'\n+\n+. ./test-lib.sh\n+\n+test_tick\n+\n+test_expect_success setup '\n+\ttest_commit file0 &&\n+\tgit tag checkpoint &&\n+\n+\techo A >fileA.t &&\n+\techo B >fileB.t &&\n+\techo C >fileC.t &&\n+\techo D >fileD.t &&\n+\tgit add fileA.t fileB.t fileC.t fileD.t\n+'\n+\n+restore_checkpoint () {\n+\tgit reset --soft checkpoint\n+}\n+\n+verify_expect () {\n+\tgit diff-tree --no-commit-id --name-status -r HEAD >actual &&\n+\ttest_cmp expect actual\n+}\n+\n+test_expect_success '--pathspec-from-file from stdin' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success '--pathspec-from-file from file' '\n+\trestore_checkpoint &&\n+\n+\techo fileA.t >list &&\n+\tgit commit --pathspec-from-file=list -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'NUL delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\0fileB.t\\0\" | git commit --pathspec-from-file=- --pathspec-file-nul -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'LF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\nfileB.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'no trailing delimiter' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\nfileB.t\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'CRLF delimiters' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileA.t\\r\\nfileB.t\\r\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tA\tfileB.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_expect_success 'quotes' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileA.t\n+\tEOF\n+\tverify_expect expect\n+'\n+\n+test_expect_success 'quotes not compatible with --pathspec-file-nul' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"\\\"file\\\\101.t\\\"\" >list &&\n+\ttest_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m \"Commit\"\n+'\n+\n+test_expect_success 'only touches what was listed' '\n+\trestore_checkpoint &&\n+\n+\tprintf \"fileB.t\\nfileC.t\\n\" | git commit --pathspec-from-file=- -m \"Commit\" &&\n+\n+\tcat >expect <<-\\EOF &&\n+\tA\tfileB.t\n+\tA\tfileC.t\n+\tEOF\n+\tverify_expect\n+'\n+\n+test_done\n-- \ngitgitgadget\n\n"},{"id":"387445","messageId":"a498dda97be76bc49677d8491457102bfb4af405.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 10/13] doc: checkout: fix broken text reference","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:15Z","receivedAt":"2019-12-03T14:02:31Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-checkout.txt | 10 ++++------\n 1 file changed, 4 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 2011fdbb1d..d47046e050 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -95,12 +95,10 @@ using `--ours` or `--theirs`. With `-m`, changes made to the working tree\n file can be discarded to re-create the original conflicted merge result.\n \n 'git checkout' (-p|--patch) [] [--] [...]::\n-\tThis is similar to the \"check out paths to the working tree\n-\tfrom either the index or from a tree-ish\" mode described\n-\tabove, but lets you use the interactive interface to show\n-\tthe \"diff\" output and choose which hunks to use in the\n-\tresult. See below for the description of `--patch` option.\n-\n+\tThis is similar to the previous mode, but lets you use the\n+\tinteractive interface to show the \"diff\" output and choose which\n+\thunks to use in the result. See below for the description of\n+\t`--patch` option.\n \n OPTIONS\n -------\n-- \ngitgitgadget\n\n"},{"id":"387446","messageId":"a97910cb55be86738d499786511db0ea9af9f785.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 07/13] cmd_add: prepare for next patch","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:12Z","receivedAt":"2019-12-03T14:02:31Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nSome code blocks were moved down to be able to test for `pathspec.nr`\nin the next patch. Blocks are moved as is without any changes. This\nis done as separate patch to reduce the amount of diffs in next patch.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/add.c | 34 +++++++++++++++++-----------------\n 1 file changed, 17 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex dd18e5c9b6..4fabdc72e6 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -418,10 +418,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (addremove && take_worktree_changes)\n \t\tdie(_(\"-A and -u are mutually incompatible\"));\n \n-\tif (!take_worktree_changes && addremove_explicit < 0 && argc)\n-\t\t/* Turn \"git add pathspec...\" to \"git add -A pathspec...\" */\n-\t\taddremove = 1;\n-\n \tif (!show_only && ignore_missing)\n \t\tdie(_(\"Option --ignore-missing can only be used together with --dry-run\"));\n \n@@ -434,19 +430,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \thold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);\n \n-\tflags = ((verbose ? ADD_CACHE_VERBOSE : 0) |\n-\t\t (show_only ? ADD_CACHE_PRETEND : 0) |\n-\t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n-\t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n-\t\t (!(addremove || take_worktree_changes)\n-\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0));\n-\n-\tif (require_pathspec && argc == 0) {\n-\t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n-\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n-\t\treturn 0;\n-\t}\n-\n \t/*\n \t * Check the \"pathspec '%s' did not match any files\" block\n \t * below before enabling new magic.\n@@ -456,6 +439,23 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t PATHSPEC_SYMLINK_LEADING_PATH,\n \t\t prefix, argv);\n \n+\tif (require_pathspec && argc == 0) {\n+\t\tfprintf(stderr, _(\"Nothing specified, nothing added.\\n\"));\n+\t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n+\t\treturn 0;\n+\t}\n+\n+\tif (!take_worktree_changes && addremove_explicit < 0 && argc)\n+\t\t/* Turn \"git add pathspec...\" to \"git add -A pathspec...\" */\n+\t\taddremove = 1;\n+\n+\tflags = ((verbose ? ADD_CACHE_VERBOSE : 0) |\n+\t\t (show_only ? ADD_CACHE_PRETEND : 0) |\n+\t\t (intent_to_add ? ADD_CACHE_INTENT : 0) |\n+\t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n+\t\t (!(addremove || take_worktree_changes)\n+\t\t ? ADD_CACHE_IGNORE_REMOVAL : 0));\n+\n \tif (read_cache_preload(&pathspec) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n-- \ngitgitgadget\n\n"},{"id":"387447","messageId":"5e449c8d29cbade125bacac6c04778ea85e3b0bd.1575381738.git.gitgitgadget@gmail.com","threadId":"52185","inReplyTo":"pull.445.v4.git.1575381738.gitgitgadget@gmail.com","subject":"[PATCH v4 09/13] doc: checkout: remove duplicate synopsis","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-12-03T14:02:14Z","receivedAt":"2019-12-03T14:02:32Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nIt was added in [1]. I understand that the duplicate change was not\nintentional and comes from an oversight.\n\nAlso, in explanation, there was only one section for two synopsis\nentries.\n\nFix both problems by removing duplicate synopsis.\n\n vs is resolved in next patch.\n\n[1] Commit b59698ae (\"checkout doc: clarify command line args for \"checkout paths\" mode\" 2017-10-11)\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n Documentation/git-checkout.txt | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex cf3cac0a2b..2011fdbb1d 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -13,7 +13,6 @@ SYNOPSIS\n 'git checkout' [-q] [-f] [-m] [--detach] \n 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] []\n 'git checkout' [-f|--ours|--theirs|-m|--conflict=