Add --pathspec-from-file option for reset, commit

Adds --pathspec-from-file option for porcelain commands to avoid commandline length limit.

So far I implemented it for git commit and git reset, but my goal is to support other commands as well after these patches are reviewed.

The patches are based on the following discussions:
https://public-inbox.org/git/[email protected]/T/#u
There, --stdin-paths was suggested.
https://public-inbox.org/git/[email protected]/T/#u
There, --stdin-paths was extended to --paths-file.
https://public-inbox.org/git/[email protected]/
https://github.com/gitgitgadget/git/pull/133Patch from Johannes, I used it as base of my patch. There, --pathspec-from-file is suggested in discussion.

Major changes compared to patch from Johannes:

1) --pathspec-from-file allows to read file, not just stdin 2) --literal-pathspecs should be honored. This is a good improvement because it keeps --pathspec-from-file much closer to passing args on commandline. Since the goal of the new option is to avoid commandline length limit, both behaviors should be close to each other. 3) Patches are designed with all other git commands in mind

Alexandr Miloslavskiy (5):
pathspec: add new function to parse file
doc: reset: unify <pathspec> description
reset: support the --pathspec-from-file option
doc: commit: unify <pathspec> description
commit: support the --pathspec-from-file option

 Documentation/git-commit.txt    |  19 ++++-
 Documentation/git-reset.txt     |  40 +++++++---
 builtin/commit.c                |  25 ++++++-
 builtin/reset.c                 |  22 +++++-
 pathspec.c                      |  41 +++++++++++
 pathspec.h                      |  10 +++
 t/t7107-reset-pathspec-file.sh  | 126 ++++++++++++++++++++++++++++++++
 t/t7526-commit-pathspec-file.sh | 107 +++++++++++++++++++++++++++
 8 files changed, 371 insertions(+), 19 deletions(-)
 create mode 100755 t/t7107-reset-pathspec-file.sh
 create mode 100755 t/t7526-commit-pathspec-file.sh

base-commit: da72936f544fec5a335e66432610e4cef4430991
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/445

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

This will be used to support the new option '--pathspec-from-file' in git add, git-commit, git reset etc.

Note also that we specifically handle CR/LF line endings to support Windows better.

To simplify code, file is first parsed into argv_array. This allows to avoid refactoring parse_pathspec().

I considered adding nul_term_line to flags instead, but decided that it doesn't fit there.

The new code is mostly taken from cmd_update_index() and split_mail_conv().

---
 pathspec.c | 41 +++++++++++++++++++++++++++++++++++++++++
 pathspec.h | 10 ++++++++++
 2 files changed, 51 insertions(+)

index 12c2b322b3..97d4e77875 100644
@@ -3,6 +3,8 @@
 #include "dir.h"
 #include "pathspec.h"
 #include "attr.h"
+#include "argv-array.h"
+#include "quote.h"
 
 /*
  * Finds which of the given pathspecs match items in the index.

@@ -613,6 +615,45 @@ void parse_pathspec(struct pathspec *pathspec,
 	}
 }
 
+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,
+			 unsigned flags, const char *prefix,
+			 const char *file, int nul_term_line)
+{
+	struct argv_array parsed_file = ARGV_ARRAY_INIT;
+	strbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :
+						       strbuf_getline;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf unquoted = STRBUF_INIT;
+	FILE *in = NULL;
+
+	if (!strcmp(file, "-"))
+		in = stdin;
+	else
+		in = fopen(file, "r");
+
+	if (!in)
+		die(_("could not open '%s' for reading"), file);
+
+	while (getline_fn(&buf, in) != EOF) {
+		if (!nul_term_line && buf.buf[0] == '"') {
+			strbuf_reset(&unquoted);
+			if (unquote_c_style(&unquoted, buf.buf, NULL))
+				die(_("line is badly quoted"));
+			strbuf_swap(&buf, &unquoted);
+		}
+		argv_array_push(&parsed_file, buf.buf);
+		strbuf_reset(&buf);
+	}
+
+	strbuf_release(&unquoted);
+	strbuf_release(&buf);
+	if (in != stdin)
+		fclose(in);
+
+	parse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);
+	argv_array_clear(&parsed_file);
+}
+
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
 {
 	int i, j;

index 1c18a2c90c..d72e0b4c4a 100644
@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,
 		    unsigned flags,
 		    const char *prefix,
 		    const char **args);
+/*
+ * Same as parse_pathspec() but uses file as input.
+ * When 'file' is exactly "-" it uses 'stdin' instead.
+ */
+void parse_pathspec_file(struct pathspec *pathspec,
+		    unsigned magic_mask,
+		    unsigned flags,
+		    const char *prefix,
+		    const char *file,
+		    int nul_term_line);
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
 void clear_pathspec(struct pathspec *);
 

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

This option solves the problem of commandline length limit for UI's built on top of git. Plumbing commands are not always a good fit, for two major reasons:
1) Some UI's serve as assistants that help user run git commands. In this case, replacing familiar commands with plumbing commands will confuse most users.
2) Some UI's have started and grown with porcelain commands. Replacing existing logic with plumbing commands could be cumbersome and prone to various new problems.

The new option is designed to behave very close to pathspecs passed in commandline args, so that switching from one to another is simple.

The new option allows to read either a specified file or stdin. Reading from file is a good way to avoid competing for stdin, and also gives some extra flexibility.

Decisions taken for simplicity:
1) The new option is declared incompatible with other options that could use stdin.
2) It is not allowed to pass some refspecs in args and others in file.
3) New options do not have shorthands to avoid shorthand conflicts.

Also add new '--pathspec-file-null' switch that mirrors '-z' used in various places. Some porcelain commands, such as git commit, already use '-z', therefore it needed a new unambiguous name.

---
 Documentation/git-reset.txt    |  18 ++++-
 builtin/reset.c                |  22 +++++-
 t/t7107-reset-pathspec-file.sh | 126 +++++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+), 5 deletions(-)
 create mode 100755 t/t7107-reset-pathspec-file.sh

index b0ea6e0ce5..d484cd2827 100644
@@ -9,18 +9,20 @@ SYNOPSIS
 --------
 [verse]
 'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-null]] [<tree-ish>]
 'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION
 -----------
-In the first and second form, copy entries from `<tree-ish>` to the index.
-In the third form, set the current branch head (`HEAD`) to `<commit>`,
+In the first three forms, copy entries from `<tree-ish>` to the index.
+In the last form, set the current branch head (`HEAD`) to `<commit>`,
 optionally modifying index and working tree to match.
 The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
 
 'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
-	This form resets the index entries for all `<pathspec>` to their
+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-null]] [<tree-ish>]::
+	These forms reset the index entries for all `<pathspec>` to their
 	state at `<tree-ish>`.  (It does not affect the working tree or
 	the current branch.)
 +

@@ -107,6 +109,16 @@ OPTIONS
 	`reset.quiet` config option. `--quiet` and `--no-quiet` will
 	override the default behavior.
 
+--pathspec-from-file=<file>::
+	Read `<pathspec>` from `<file>` instead. If `<file>` is exactly `-`
+	then read from standard input. Pathspecs are separated by LF or
+	CR/LF. Pathspecs can be quoted as explained for the configuration
+	variable `core.quotePath` (see linkgit:git-config[1]). See also
+	`--pathspec-file-null` and global `--literal-pathspecs`.
+
+--pathspec-file-null::
+	Only meaningful with `--pathspec-from-file`. Pathspecs are
+	separated with NUL character and are not expected to be quoted.
 
 EXAMPLES
 --------

index fdd572168b..0eaa6b0bca 100644
@@ -31,6 +31,7 @@
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+	N_("git reset [-q] [--pathspec-from-file [--pathspec-file-null]] [<tree-ish>]"),
 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
 	NULL
 };

@@ -284,8 +285,8 @@ static int git_reset_config(const char *var, const char *value, void *cb)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0, unborn;
-	const char *rev;
+	int patch_mode = 0, pathspec_file_null = 0, unborn;
+	const char *rev, *pathspec_from_file = NULL;
 	struct object_id oid;
 	struct pathspec pathspec;
 	int intent_to_add = 0;

@@ -306,6 +307,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
+		OPT_FILENAME(0, "pathspec-from-file", &pathspec_from_file,
+				N_("read pathspecs from file")),
+		OPT_BOOL(0, "pathspec-file-null", &pathspec_file_null,
+				N_("with --pathspec-from-file, pathspecs are separated with NUL character")),
 		OPT_END()
 	};
 

@@ -316,6 +321,19 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	if (pathspec_from_file) {
+		if (patch_mode)
+			die(_("--pathspec-from-file is incompatible with --patch"));
+
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with path arguments"));
+
+		parse_pathspec_file(&pathspec, 0,
+				    PATHSPEC_PREFER_FULL,
+				    prefix, pathspec_from_file, pathspec_file_null);
+	} else if (pathspec_file_null)
+		die(_("--pathspec-file-null requires --pathspec-from-file"));
+
 	unborn = !strcmp(rev, "HEAD") && get_oid("HEAD", &oid);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Synchronize it to git add, which has a pretty good description. This also better disambiguates <file>... header.

---
 Documentation/git-commit.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

index afa7b75a23..4341d0e3ab 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<file>...]
+	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -345,12 +345,15 @@ changes to tracked files.
 \--::
 	Do not interpret any more arguments as options.
 
-<file>...::
+<pathspec>...::
 	When files are given on the command line, the command
 	commits the contents of the named files, without
 	recording the changes already staged.  The contents of
 	these files are also staged for the next commit on top
 	of what have been staged before.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
 
 :git-commit: 1
 include::date-formats.txt[]

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

This option solves the problem of commandline length limit for UI's built on top of git. Plumbing commands are not always a good fit, for two major reasons:
1) Some UI's serve as assistants that help user run git commands. In this case, replacing familiar commands with plumbing commands will confuse most users.
2) Some UI's have started and grown with porcelain commands. Replacing existing logic with plumbing commands could be cumbersome and prone to various new problems.

The new option is designed to behave very close to pathspecs passed in commandline args, so that switching from one to another is simple.

The new option allows to read either a specified file or stdin. Reading from file is a good way to avoid competing for stdin, and also gives some extra flexibility.

Decisions taken for simplicity:
1) The new option is declared incompatible with other options that could use stdin.
2) It is not allowed to pass some refspecs in args and others in file.
3) New options do not have shorthands to avoid shorthand conflicts.

Also add new '--pathspec-file-null' switch that mirrors '-z' used in various places. Some porcelain commands, such as git commit, already use '-z', therefore it needed a new unambiguous name.

---
 Documentation/git-commit.txt    |  14 ++++-
 builtin/commit.c                |  25 ++++++--
 t/t7526-commit-pathspec-file.sh | 107 ++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+), 5 deletions(-)
 create mode 100755 t/t7526-commit-pathspec-file.sh

index 4341d0e3ab..ec4752298d 100644
@@ -13,7 +13,8 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
+	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-null]]
+	   [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -277,6 +278,17 @@ FROM UPSTREAM REBASE" section in linkgit:git-rebase[1].)
 	the last commit without committing changes that have
 	already been staged. If used together with `--allow-empty`
 	paths are also not required, and an empty commit will be created.
+	
+--pathspec-from-file=<file>::
+	Read `<pathspec>` from `<file>` instead. If `<file>` is exactly `-`
+	then read from standard input. Pathspecs are separated by LF or
+	CR/LF. Pathspecs can be quoted as explained for the configuration
+	variable `core.quotePath` (see linkgit:git-config[1]). See also
+	`--pathspec-file-null` and global `--literal-pathspecs`.
+
+--pathspec-file-null::
+	Only meaningful with `--pathspec-from-file`. Pathspecs are
+	separated with NUL character and are not expected to be quoted.
 
 -u[<mode>]::
 --untracked-files[=<mode>]::

index e588bc6ad3..532f305926 100644
@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
 static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
-static int no_post_rewrite, allow_empty_message;
+static int no_post_rewrite, allow_empty_message, pathspec_file_null;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
-static char *sign_commit;
+static char *sign_commit, *pathspec_from_file;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
 
+	if (pathspec_from_file) {
+		if (interactive)
+			die(_("--pathspec-from-file is incompatible with --interactive/--patch"));
+
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with path arguments"));
+
+		parse_pathspec_file(&pathspec, 0,
+				    PATHSPEC_PREFER_FULL,
+				    prefix, pathspec_from_file, pathspec_file_null);
+	}
+	else if (pathspec_file_null)
+		die(_("--pathspec-file-null requires --pathspec-from-file"));
+
+	if (!pathspec.nr && (also || (only && !amend && !allow_empty)))
+		die(_("No paths with --include/--only does not make sense."));
+
 	if (read_cache_preload(&pathspec) < 0)
 		die(_("index file corrupt"));
 

@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
-	if (argc == 0 && (also || (only && !amend && !allow_empty)))
-		die(_("No paths with --include/--only does not make sense."));
 	cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);
 
 	handle_untracked_files_arg(s);

@@ -1535,6 +1550,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
 		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
 		{ 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" },
+		OPT_FILENAME(0, "pathspec-from-file", &pathspec_from_file, N_("read pathspecs from file")),
+		OPT_BOOL(0, "pathspec-file-null", &pathspec_file_null, N_("with --pathspec-from-file, pathspecs are separated with NUL character")),
 		/* end commit contents options */
 
 		OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Synchronize it to git add, which has a pretty good description.

---
 Documentation/git-reset.txt | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

index 97e0544d9e..b0ea6e0ce5 100644
@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [-q] [<tree-ish>] [--] <paths>...
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION

@@ -19,27 +19,33 @@ In the third form, set the current branch head (`HEAD`) to `<commit>`,
 optionally modifying index and working tree to match.
 The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
 
-'git reset' [-q] [<tree-ish>] [--] <paths>...::
-	This form resets the index entries for all `<paths>` to their
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
+	This form resets the index entries for all `<pathspec>` to their
 	state at `<tree-ish>`.  (It does not affect the working tree or
 	the current branch.)
 +
-This means that `git reset <paths>` is the opposite of `git add
-<paths>`. This command is equivalent to
-`git restore [--source=<tree-ish>] --staged <paths>...`.
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
 +
-After running `git reset <paths>` to update the index entry, you can
+This means that `git reset <pathspec>` is the opposite of `git add
+<pathspec>`. This command is equivalent to
+`git restore [--source=<tree-ish>] --staged <pathspec>...`.
++
+After running `git reset <pathspec>` to update the index entry, you can
 use linkgit:git-restore[1] to check the contents out of the index to
 the working tree. Alternatively, using linkgit:git-restore[1]
 and specifying a commit with `--source`, you
 can copy the contents of a path out of a commit to the index and to the
 working tree in one go.
 
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]::
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]::
 	Interactively select hunks in the difference between the index
 	and `<tree-ish>` (defaults to `HEAD`).  The chosen hunks are applied
 	in reverse to the index.
 +
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
++
 This means that `git reset -p` is the opposite of `git add -p`, i.e.
 you can use it to selectively reset hunks. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.

gitgitgadget

Hi Alexandr

Thanks for working on this. I've got a couple of comments about improving the error messages but this looks fine to me

Hi Alexandr

This also looks good, I've got some minor comments below. If I'm complaining about whitespace and style issues you know the patch is quite good!

I think reset is a good example of the second reason - it's not straight forward to implement it in plumbing commands

I think this flexibility is a good idea

I'm confused reset does not use stdin does it?

s/refspecs/pathspecs/ ?

As the 'lines' in the file are nul terminated perhaps it would be better to call this --pathspec-file-nul or --nul-termination. I think the use of --null to mean nul termination for config was a mistake (for grep it matches what GUN grep does but it's still unfortunate).

Hi Alexandr

Hi Alexandr

It might be worth tailoring the message to the command rather than having exactly the same message for commit and reset

Thanks for the pointer!

Good idea.

Sorry, still struggling with my VS that tries to do crazy things. Will fix.

Hi Alexandr

Oh I think I've misunderstood. The '\\' in unquoted by printf, so git sees '\101' which is A so that is a real file.

Sorry for the confusion

Phillip

Thanks for your support :) Previously the opinions were mixed and I was a bit afraid that this will invoke a new round of discussions.

I understand that '--patch' interacts with user via stdin. Will double-check tomorrow.

Thanks! Not quite used to git speak and mix up things sometimes.

OK, will change to '--pathspec-file-nul'

'--pathspec-from-file' was kind of suggested instead of '--paths-file' by Junio here:
https://public-inbox.org/git/[email protected]/

so Junio added '-from' to previous writing. Hmm. What do you think, taking Junio's message into account?

As for whitespaces, sorry, will fix.

Will fix

Shall I replace '<pathspec>' with 'pathspecs' ?

OK

Yes, I also thought so. I doubt that user is willing to interactively decide on hundreds of files.

Took this code from a guy who ignored it :) But sure, will change.

I'll try to change this.

In my understanding, 'verify_state expect.a' should test that it's correctly understood. Am I wrong?

Good idea.

I thought that whole test package is about '--pathspec-from-file' so I'd rather not repeat that in every test name. Shall I change that?

Thanks for the pointer!

I also was somewhat unhappy about duplication. But I didn't figure how to do that correctly. Currently the messages for 'git reset' and 'git commit' are almost identical.

Maybe in 2nd commit I should say something like "Extend --pathspec-file-null support to git commit (see previous patch for git reset)" ?

Yes, sure, I will try to apply your suggestions to all patches. Hopefully without forgetting things :)

Yes, I was also somewhat unhappy about that. I will give it more thought.

Thanks, and also thanks for reviewing my patches!

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

Any time I see "... X. This means Y." either in the doc or in the proposed log message, I wish the author (not you in this case, obviously) thought twice about rewriting so that they do not say one thing and immediately have to rephrase it, i.e. either just say Y without saying X, or saying X more clearly without having to say Y.

In this case, however, I think X and Y are related but both relevant. The subcommand resets the index entries for chosen paths to match what is in the tree-ish, which is the same as restoring from a tree to the index.

It is not quite the opposite of adding to the index from the working tree. In this sequence:

	$ edit newfile
	$ git add newfile

and then further

	$ edit newfile
	$ git add newfile
	$ git reset -- newfile

we are taken back to the state _before_ any of the changes made to newfile (in fact, since HEAD does not have newfile, the resulting index would not know about it, either).

Thanks.

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

"UI's that serve as assistants that help user run git commands" does not have to avoid plumbing commands at all. Only the ones that "show" the commands that are run on behalf of the users (perhaps so that the users can learn from such examples?) do, and I think I learned that it was your motivating use case from an earlier discussion. Perhaps

UIs that help users to formulate git commands to run need to present Porcelain commands to be used, as it is not reasonable to demonstrate arcane combination of plumbing commands as an example for their interactive use.

would probably be more readable without bending what you wanted to say too much?

There is not a lot of sympathy for such argument ;-)

Did you mean refspec, not pathspec?

I do not understand this part. Wouldn't it natural to expect that "-z", when used with "--pathspec-from-file", tell us that the named file is NUL delimited collection of records? What's different about "--pathspec-file-null" that it is confusing to all it "-z"?

I actually do not mind not having "-z" and using only "--pathspec-file-null". A new option with long name that feels similar to existing "-z" but does sufficiently different things so that it needs a different name, however, feels counter-productive for the purpose of using it in the UI that produces commands to be learned by end-users.

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

"When files are given on..." is no longer true with this change (it wasn't true with the code before this change anyway).

When pathspec is given on the command line, commit the contents of the files that match the pathspec without recording the changes already added to the index. ...

The second sentence also says "these files", but that can be left as-is, since it would refer to "the files that match ..." explained in the above sentence.

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

I think all the comments I made on "reset" with these two options, both to the proposed log message and to the patch text, applies to this this step, too, so I won't repeat them.

Thanks for working on this topic.

From: Alexandr Miloslavskiy <[email protected]>

Support for various porcelain commands will arrive via additional patches.

--pathspec-from-file solves the problem of commandline length limit for UIs built on top of git. Plumbing commands are not always a good fit, for two major reasons:
1) Some UIs show executed commands to user. In this case, porcelain commands are expected. One reason for that is letting user learn git commands by clicking UI buttons. The other reason is letting user study the history of commands in case of any unexpected results. Both of these will lose most of their value if UI uses combinations of arcane plumbing commands.
2) Some UIs have started and grown with porcelain commands. Replacing existing logic with plumbing commands could be cumbersome and prone to various new problems.

--pathspec-from-file will behave very close to pathspec passed in commandline args, so that switching from one to another is simple.

--pathspec-from-file will read either a specified file or stdin (when file is exactly "-"). Reading from file is a good way to avoid competing for stdin, and also gives some extra flexibility.

--pathspec-file-nul switch mirrors -z already used in various places. Some porcelain commands, such as git commit, already use -z, therefore it needed a new unambiguous name.

New options do not have shorthands to avoid shorthand conflicts. It is not expected that they will be typed in console.

---
 parse-options.h | 2 ++
 1 file changed, 2 insertions(+)

index 38a33a087e..c6cc01e715 100644
@@ -330,5 +330,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 #define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 #define OPT_CLEANUP(v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))
+#define OPT_PATHSPEC_FROM_FILE(v) OPT_FILENAME(0, "pathspec-from-file", v, N_("read pathspec from file"))
+#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"))
 
 #endif

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

This will be used to support the new option '--pathspec-from-file' in git add, git-commit, git reset etc.

Note also that we specifically handle CR/LF line endings to support Windows better.

To simplify code, file is first parsed into argv_array. This allows to avoid refactoring parse_pathspec().

I considered adding nul_term_line to flags instead, but decided that it doesn't fit there.

The new code is mostly taken from cmd_update_index() and split_mail_conv().

---
 pathspec.c | 38 ++++++++++++++++++++++++++++++++++++++
 pathspec.h | 10 ++++++++++
 2 files changed, 48 insertions(+)

index 12c2b322b3..2e9b376318 100644
@@ -3,6 +3,8 @@
 #include "dir.h"
 #include "pathspec.h"
 #include "attr.h"
+#include "argv-array.h"
+#include "quote.h"
 
 /*
  * Finds which of the given pathspecs match items in the index.

@@ -613,6 +615,42 @@ void parse_pathspec(struct pathspec *pathspec,
 	}
 }
 
+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,
+			 unsigned flags, const char *prefix,
+			 const char *file, int nul_term_line)
+{
+	struct argv_array parsed_file = ARGV_ARRAY_INIT;
+	strbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :
+						       strbuf_getline;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf unquoted = STRBUF_INIT;
+	FILE *in = NULL;
+
+	if (!strcmp(file, "-"))
+		in = stdin;
+	else
+		in = xfopen(file, "r");
+
+	while (getline_fn(&buf, in) != EOF) {
+		if (!nul_term_line && buf.buf[0] == '"') {
+			strbuf_reset(&unquoted);
+			if (unquote_c_style(&unquoted, buf.buf, NULL))
+				die(_("line is badly quoted: %s"), buf.buf);
+			strbuf_swap(&buf, &unquoted);
+		}
+		argv_array_push(&parsed_file, buf.buf);
+		strbuf_reset(&buf);
+	}
+
+	strbuf_release(&unquoted);
+	strbuf_release(&buf);
+	if (in != stdin)
+		fclose(in);
+
+	parse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);
+	argv_array_clear(&parsed_file);
+}
+
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
 {
 	int i, j;

index 1c18a2c90c..a27dc81ba2 100644
@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,
 		    unsigned flags,
 		    const char *prefix,
 		    const char **args);
+/*
+ * Same as parse_pathspec() but uses file as input.
+ * When 'file' is exactly "-" it uses 'stdin' instead.
+ */
+void parse_pathspec_file(struct pathspec *pathspec,
+			 unsigned magic_mask,
+			 unsigned flags,
+			 const char *prefix,
+			 const char *file,
+			 int nul_term_line);
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
 void clear_pathspec(struct pathspec *);
 

gitgitgadget

Adds --pathspec-from-file option for porcelain commands to avoid commandline length limit.

So far I implemented it for git commit and git reset, but my goal is to support other commands as well after these patches are reviewed.

The patches are based on the following discussions:
https://public-inbox.org/git/[email protected]/T/#u
There, --stdin-paths was suggested.
https://public-inbox.org/git/[email protected]/T/#u
There, --stdin-paths was extended to --paths-file.
https://public-inbox.org/git/[email protected]/
https://github.com/gitgitgadget/git/pull/133Patch from Johannes, I used it as base of my patch. There, --pathspec-from-file is suggested in discussion.

Major changes compared to patch from Johannes:

1) --pathspec-from-file allows to read file, not just stdin

2) --literal-pathspecs should be honored. This is a good improvement because it keeps --pathspec-from-file much closer to passing args on commandline. Since the goal of the new option is to avoid commandline length limit, both behaviors should be close to each other.

3) Patches are designed with all other git commands in mind

Changes from V1: A lot of small changes here and there, in accordance with review suggestions.

Alexandr Miloslavskiy (6):
parse-options.h: add new options --pathspec-from-file,

    `--pathspec-file-nul`

pathspec: add new function to parse file
doc: reset: unify <pathspec> description
reset: support the --pathspec-from-file option
doc: commit: unify <pathspec> description
commit: support the --pathspec-from-file option

 Documentation/git-commit.txt    |  29 ++++--
 Documentation/git-reset.txt     |  37 +++++---
 builtin/commit.c                |  25 +++++-
 builtin/reset.c                 |  25 +++++-
 parse-options.h                 |   2 +
 pathspec.c                      |  38 ++++++++
 pathspec.h                      |  10 +++
 t/t7107-reset-pathspec-file.sh  | 155 ++++++++++++++++++++++++++++++++
 t/t7526-commit-pathspec-file.sh | 130 +++++++++++++++++++++++++++
 9 files changed, 425 insertions(+), 26 deletions(-)
 create mode 100755 t/t7107-reset-pathspec-file.sh
 create mode 100755 t/t7526-commit-pathspec-file.sh

base-commit: da72936f544fec5a335e66432610e4cef4430991
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/445

Range-diff vs v1:

 -:  ---------- > 1:  2dfaccf0d5 parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`
 1:  52e7a84a2e ! 2:  96697ba072 pathspec: add new function to parse file
     @@ -50,16 +50,13 @@
      +	if (!strcmp(file, "-"))
      +		in = stdin;
      +	else
     -+		in = fopen(file, "r");
     -+
     -+	if (!in)
     -+		die(_("could not open '%s' for reading"), file);
     ++		in = xfopen(file, "r");
      +
      +	while (getline_fn(&buf, in) != EOF) {
      +		if (!nul_term_line && buf.buf[0] == '"') {
      +			strbuf_reset(&unquoted);
      +			if (unquote_c_style(&unquoted, buf.buf, NULL))
     -+				die(_("line is badly quoted"));
     ++				die(_("line is badly quoted: %s"), buf.buf);
      +			strbuf_swap(&buf, &unquoted);
      +		}
      +		argv_array_push(&parsed_file, buf.buf);
     @@ -91,11 +88,11 @@
      + * When 'file' is exactly "-" it uses 'stdin' instead.
      + */
      +void parse_pathspec_file(struct pathspec *pathspec,
     -+		    unsigned magic_mask,
     -+		    unsigned flags,
     -+		    const char *prefix,
     -+		    const char *file,
     -+		    int nul_term_line);
     ++			 unsigned magic_mask,
     ++			 unsigned flags,
     ++			 const char *prefix,
     ++			 const char *file,
     ++			 int nul_term_line);
       void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
       void clear_pathspec(struct pathspec *);

2: 1740ac7a29 ! 3: f961a5155a doc: reset: unify <pathspec> description
@@ -21,8 +21,11 @@

DESCRIPTION
@@
+ In the third form, set the current branch head (HEAD) to <commit>, optionally modifying index and working tree to match.
The <tree-ish>/<commit> defaults to HEAD in all forms.
++The <pathspec> is used to limit the paths affected by the operation ++(see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).

-'git reset' [-q] [<tree-ish>] [--] <paths>...::
- This form resets the index entries for all <paths> to their
@@ -34,14 +37,11 @@
-This means that git reset <paths> is the opposite of git add -<paths>. This command is equivalent to
-git restore [--source=<tree-ish>] --staged <paths>....
-+For more details about the <pathspec> syntax, see the 'pathspec' entry -+in linkgit:gitglossary[7].
- +
--After running git reset <paths> to update the index entry, you can
+This means that git reset <pathspec> is the opposite of git add +<pathspec>. This command is equivalent to
+git restore [--source=<tree-ish>] --staged <pathspec>....
-++
+ +
+-After running git reset <paths> to update the index entry, you can
+After running git reset <pathspec> to update the index entry, you can
use linkgit:git-restore[1] to check the contents out of the index to the working tree. Alternatively, using linkgit:git-restore[1]
@@ -54,10 +54,18 @@
Interactively select hunks in the difference between the index and <tree-ish> (defaults to HEAD). The chosen hunks are applied in reverse to the index.
- +
-+For more details about the <pathspec> syntax, see the 'pathspec' entry -+in linkgit:gitglossary[7].
-++
- This means that git reset -p is the opposite of git add -p, i.e.
- you can use it to selectively reset hunks. See the ``Interactive Mode''
- section of linkgit:git-add[1] to learn how to operate the --patch mode. +
+ diff --git a/builtin/reset.c b/builtin/reset.c
+ --- a/builtin/reset.c
+ +++ b/builtin/reset.c
+@@
+
+ static const char * const git_reset_usage[] = {
+ N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
+- N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
+- N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
++ N_("git reset [-q] [<tree-ish>] [--] <pathspec>..."),
++ N_("git reset --patch [<tree-ish>] [--] [<pathspec>...]"),
+ NULL
+ };
+
3: 8d9f1fbc18 ! 4: d72d4f16b5 reset: support the --pathspec-from-file option

     @@ -1,33 +1,13 @@
      Author: Alexandr Miloslavskiy <[email protected]>
      
     -    reset: support the --pathspec-from-file option
     -
     -    This option solves the problem of commandline length limit for UI's
     -    built on top of git. Plumbing commands are not always a good fit, for
     -    two major reasons:
     -    1) Some UI's serve as assistants that help user run git commands. In
     -       this case, replacing familiar commands with plumbing commands will
     -       confuse most users.
     -    2) Some UI's have started and grown with porcelain commands. Replacing
     -       existing logic with plumbing commands could be cumbersome and prone
     -       to various new problems.
     -
     -    The new option is designed to behave very close to pathspecs passed in
     -    commandline args, so that switching from one to another is simple.
     -
     -    The new option allows to read either a specified file or `stdin`.
     -    Reading from file is a good way to avoid competing for stdin, and
     -    also gives some extra flexibility.
     +    reset: support the `--pathspec-from-file` option
      
          Decisions taken for simplicity:
     -    1) The new option is declared incompatible with other options that
     -       could use `stdin`.
     -    2) It is not allowed to pass some refspecs in args and others in file.
     -    3) New options do not have shorthands to avoid shorthand conflicts.
     -
     -    Also add new '--pathspec-file-null' switch that mirrors '-z' used in
     -    various places. Some porcelain commands, such as `git commit`, already
     -    use '-z', therefore it needed a new unambiguous name.
     +    1) For now, `--pathspec-from-file` is declared incompatible with
     +       `--patch`, even when <file> is not `stdin`. Such use case it not
     +       really expected. Also, it is harder to support in `git commit`, so
     +       I decided to make it incompatible in all places.
     +    2) It is not allowed to pass pathspec in both args and file.
      
          Co-authored-by: Johannes Schindelin <[email protected]>
          Signed-off-by: Alexandr Miloslavskiy <[email protected]>
     @@ -39,7 +19,7 @@
       --------
       [verse]
       'git reset' [-q] [<tree-ish>] [--] <pathspec>...
     -+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-null]] [<tree-ish>]
     ++'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-nul]] [<tree-ish>]
       'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
       'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
       
     @@ -51,11 +31,13 @@
      +In the last form, set the current branch head (`HEAD`) to `<commit>`,
       optionally modifying index and working tree to match.
       The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
     + The <pathspec> is used to limit the paths affected by the operation
     + (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).
       
       'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
      -	This form resets the index entries for all `<pathspec>` to their
     -+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-null]] [<tree-ish>]::
     -+	These forms reset the index entries for all `<pathspec>` to their
     ++'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-nul]] [<tree-ish>]::
     ++	These forms reset the index entries matching pathspec to their
       	state at `<tree-ish>`.  (It does not affect the working tree or
       	the current branch.)
       +
     @@ -64,14 +46,15 @@
       	override the default behavior.
       
      +--pathspec-from-file=<file>::
     -+	Read `<pathspec>` from `<file>` instead. If `<file>` is exactly `-`
     -+	then read from standard input. Pathspecs are separated by LF or
     -+	CR/LF. Pathspecs can be quoted as explained for the configuration
     -+	variable `core.quotePath` (see linkgit:git-config[1]). See also
     -+	`--pathspec-file-null` and global `--literal-pathspecs`.
     -+
     -+--pathspec-file-null::
     -+	Only meaningful with `--pathspec-from-file`. Pathspecs are
     ++	Pathspec is passed in `<file>` instead of commandline args. If
     ++	`<file>` is exactly `-` then standard input is used. Pathspec
     ++	elements are separated by LF or CR/LF. Pathspec elements can be
     ++	quoted as explained for the configuration variable `core.quotePath`
     ++	(see linkgit:git-config[1]). See also `--pathspec-file-nul` and
     ++	global `--literal-pathspecs`.
     ++
     ++--pathspec-file-nul::
     ++	Only meaningful with `--pathspec-from-file`. Pathspec elements are
      +	separated with NUL character and are not expected to be quoted.
       
       EXAMPLES
     @@ -83,9 +66,9 @@
      @@
       static const char * const git_reset_usage[] = {
       	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
     - 	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
     -+	N_("git reset [-q] [--pathspec-from-file [--pathspec-file-null]] [<tree-ish>]"),
     - 	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
     + 	N_("git reset [-q] [<tree-ish>] [--] <pathspec>..."),
     ++	N_("git reset [-q] [--pathspec-from-file [--pathspec-file-nul]] [<tree-ish>]"),
     + 	N_("git reset --patch [<tree-ish>] [--] [<pathspec>...]"),
       	NULL
       };
      @@
     @@ -94,7 +77,7 @@
       	int reset_type = NONE, update_ref_status = 0, quiet = 0;
      -	int patch_mode = 0, unborn;
      -	const char *rev;
     -+	int patch_mode = 0, pathspec_file_null = 0, unborn;
     ++	int patch_mode = 0, pathspec_file_nul = 0, unborn;
      +	const char *rev, *pathspec_from_file = NULL;
       	struct object_id oid;
       	struct pathspec pathspec;
     @@ -103,10 +86,8 @@
       		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
       		OPT_BOOL('N', "intent-to-add", &intent_to_add,
       				N_("record only the fact that removed paths will be added later")),
     -+		OPT_FILENAME(0, "pathspec-from-file", &pathspec_from_file,
     -+				N_("read pathspecs from file")),
     -+		OPT_BOOL(0, "pathspec-file-null", &pathspec_file_null,
     -+				N_("with --pathspec-from-file, pathspecs are separated with NUL character")),
     ++		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
     ++		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
       		OPT_END()
       	};
       
     @@ -119,13 +100,14 @@
      +			die(_("--pathspec-from-file is incompatible with --patch"));
      +
      +		if (pathspec.nr)
     -+			die(_("--pathspec-from-file is incompatible with path arguments"));
     ++			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
      +
      +		parse_pathspec_file(&pathspec, 0,
      +				    PATHSPEC_PREFER_FULL,
     -+				    prefix, pathspec_from_file, pathspec_file_null);
     -+	} else if (pathspec_file_null)
     -+		die(_("--pathspec-file-null requires --pathspec-from-file"));
     ++				    prefix, pathspec_from_file, pathspec_file_nul);
     ++	} else if (pathspec_file_nul) {
     ++		die(_("--pathspec-file-nul requires --pathspec-from-file"));
     ++	}
      +
       	unborn = !strcmp(rev, "HEAD") && get_oid("HEAD", &oid);
       	if (unborn) {
     @@ -142,21 +124,7 @@
      +
      +. ./test-lib.sh
      +
     -+cat > expect.a <<EOF
     -+ D fileA.t
     -+EOF
     -+
     -+cat > expect.ab <<EOF
     -+ D fileA.t
     -+ D fileB.t
     -+EOF
     -+
     -+cat > expect.a_bc_d <<EOF
     -+D  fileA.t
     -+ D fileB.t
     -+ D fileC.t
     -+D  fileD.t
     -+EOF
     ++test_tick
      +
      +test_expect_success setup '
      +	echo A >fileA.t &&
     @@ -165,16 +133,16 @@
      +	echo D >fileD.t &&
      +	git add . &&
      +	git commit --include . -m "Commit" &&
     -+	checkpoint=$(git rev-parse --verify HEAD)
     ++	git tag checkpoint
      +'
      +
      +restore_checkpoint () {
     -+	git reset --hard "$checkpoint"
     ++	git reset --hard checkpoint
      +}
      +
     -+verify_state () {
     ++verify_expect () {
      +	git status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual &&
     -+	test_cmp "$1" actual
     ++	test_cmp expect actual
      +}
      +
      +test_expect_success '--pathspec-from-file from stdin' '
     @@ -183,7 +151,10 @@
      +	git rm fileA.t &&
      +	echo fileA.t | git reset --pathspec-from-file=- &&
      +
     -+	verify_state expect.a
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success '--pathspec-from-file from file' '
     @@ -193,34 +164,62 @@
      +	echo fileA.t >list &&
      +	git reset --pathspec-from-file=list &&
      +
     -+	verify_state expect.a
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'NUL delimiters' '
      +	restore_checkpoint &&
      +
      +	git rm fileA.t fileB.t &&
     -+	printf fileA.tQfileB.t | q_to_nul | git reset --pathspec-from-file=- --pathspec-file-null &&
     ++	printf "fileA.t\0fileB.t\0" | git reset --pathspec-from-file=- --pathspec-file-nul &&
      +
     -+	verify_state expect.ab
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	 D fileB.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'LF delimiters' '
      +	restore_checkpoint &&
      +
      +	git rm fileA.t fileB.t &&
     ++	printf "fileA.t\nfileB.t\n" | git reset --pathspec-from-file=- &&
     ++
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	 D fileB.t
     ++	EOF
     ++	verify_expect
     ++'
     ++
     ++test_expect_success 'no trailing delimiter' '
     ++	restore_checkpoint &&
     ++
     ++	git rm fileA.t fileB.t &&
      +	printf "fileA.t\nfileB.t" | git reset --pathspec-from-file=- &&
      +
     -+	verify_state expect.ab
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	 D fileB.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'CRLF delimiters' '
      +	restore_checkpoint &&
      +
      +	git rm fileA.t fileB.t &&
     -+	printf "fileA.t\r\nfileB.t" | git reset --pathspec-from-file=- &&
     ++	printf "fileA.t\r\nfileB.t\r\n" | git reset --pathspec-from-file=- &&
      +
     -+	verify_state expect.ab
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	 D fileB.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'quotes' '
     @@ -229,21 +228,27 @@
      +	git rm fileA.t &&
      +	printf "\"file\\101.t\"" | git reset --pathspec-from-file=- &&
      +
     -+	verify_state expect.a
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	EOF
     ++	verify_expect
      +'
      +
     -+test_expect_success 'quotes not compatible with --pathspec-file-null' '
     ++test_expect_success 'quotes not compatible with --pathspec-file-nul' '
      +	restore_checkpoint &&
      +
      +	git rm fileA.t &&
      +	printf "\"file\\101.t\"" >list &&
      +	# Note: "git reset" has not yet learned to fail on wrong pathspecs
     -+	git reset --pathspec-from-file=list --pathspec-file-null &&
     ++	git reset --pathspec-from-file=list --pathspec-file-nul &&
      +	
     -+	test_must_fail verify_state expect.a
     ++	cat >expect <<-\EOF &&
     ++	 D fileA.t
     ++	EOF
     ++	test_must_fail verify_expect
      +'
      +
     -+test_expect_success '--pathspec-from-file is not compatible with --soft --hard' '
     ++test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' '
      +	restore_checkpoint &&
      +
      +	git rm fileA.t &&
     @@ -256,9 +261,15 @@
      +	restore_checkpoint &&
      +
      +	git rm fileA.t fileB.t fileC.t fileD.t &&
     -+	printf "fileB.t\nfileC.t" | git reset --pathspec-from-file=- &&
     -+
     -+	verify_state expect.a_bc_d
     ++	printf "fileB.t\nfileC.t\n" | git reset --pathspec-from-file=- &&
     ++
     ++	cat >expect <<-\EOF &&
     ++	D  fileA.t
     ++	 D fileB.t
     ++	 D fileC.t
     ++	D  fileD.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_done

4: 251d06e27f ! 5: 20c4495fd3 doc: commit: unify <pathspec> description
@@ -24,12 +24,16 @@
Do not interpret any more arguments as options.

-<file>...::
+- When files are given on the command line, the command
+- commits the contents of the named files, without
+- recording the changes already staged. The contents of
+- these files are also staged for the next commit on top
+- of what have been staged before.
+<pathspec>...::
- When files are given on the command line, the command
- commits the contents of the named files, without
- recording the changes already staged. The contents of
- these files are also staged for the next commit on top
- of what have been staged before.
++ When pathspec is given on the command line, commit the contents of
++ the files that match the pathspec without recording the changes
++ already added to the index. The contents of these files are also
++ staged for the next commit on top of what have been staged before.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry +in linkgit:gitglossary[7].
5: f484704689 ! 6: cb5fc9b14d commit: support the --pathspec-from-file option

     @@ -2,32 +2,12 @@
      
          commit: support the --pathspec-from-file option
      
     -    This option solves the problem of commandline length limit for UI's
     -    built on top of git. Plumbing commands are not always a good fit, for
     -    two major reasons:
     -    1) Some UI's serve as assistants that help user run git commands. In
     -       this case, replacing familiar commands with plumbing commands will
     -       confuse most users.
     -    2) Some UI's have started and grown with porcelain commands. Replacing
     -       existing logic with plumbing commands could be cumbersome and prone
     -       to various new problems.
     -
     -    The new option is designed to behave very close to pathspecs passed in
     -    commandline args, so that switching from one to another is simple.
     -
     -    The new option allows to read either a specified file or `stdin`.
     -    Reading from file is a good way to avoid competing for stdin, and
     -    also gives some extra flexibility.
     -
          Decisions taken for simplicity:
     -    1) The new option is declared incompatible with other options that
     -       could use `stdin`.
     -    2) It is not allowed to pass some refspecs in args and others in file.
     -    3) New options do not have shorthands to avoid shorthand conflicts.
     -
     -    Also add new '--pathspec-file-null' switch that mirrors '-z' used in
     -    various places. Some porcelain commands, such as `git commit`, already
     -    use '-z', therefore it needed a new unambiguous name.
     +    1) For now, `--pathspec-from-file` is declared incompatible with
     +       `--interactive/--patch`, even when <file> is not `stdin`. Such use
     +       case it not really expected. Also, it would require changes to
     +       `interactive_add()`.
     +    2) It is not allowed to pass pathspec in both args and file.
      
          Signed-off-by: Alexandr Miloslavskiy <[email protected]>
      
     @@ -39,7 +19,7 @@
       	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
       	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
      -	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
     -+	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-null]]
     ++	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
      +	   [-S[<keyid>]] [--] [<pathspec>...]
       
       DESCRIPTION
     @@ -50,14 +30,15 @@
       	paths are also not required, and an empty commit will be created.
      +	
      +--pathspec-from-file=<file>::
     -+	Read `<pathspec>` from `<file>` instead. If `<file>` is exactly `-`
     -+	then read from standard input. Pathspecs are separated by LF or
     -+	CR/LF. Pathspecs can be quoted as explained for the configuration
     -+	variable `core.quotePath` (see linkgit:git-config[1]). See also
     -+	`--pathspec-file-null` and global `--literal-pathspecs`.
     -+
     -+--pathspec-file-null::
     -+	Only meaningful with `--pathspec-from-file`. Pathspecs are
     ++	Pathspec is passed in `<file>` instead of commandline args. If
     ++	`<file>` is exactly `-` then standard input is used. Pathspec
     ++	elements are separated by LF or CR/LF. Pathspec elements can be
     ++	quoted as explained for the configuration variable `core.quotePath`
     ++	(see linkgit:git-config[1]). See also `--pathspec-file-nul` and
     ++	global `--literal-pathspecs`.
     ++
     ++--pathspec-file-nul::
     ++	Only meaningful with `--pathspec-from-file`. Pathspec elements are
      +	separated with NUL character and are not expected to be quoted.
       
       -u[<mode>]::
     @@ -71,7 +52,7 @@
       static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
       static int config_commit_verbose = -1; /* unspecified */
      -static int no_post_rewrite, allow_empty_message;
     -+static int no_post_rewrite, allow_empty_message, pathspec_file_null;
     ++static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
       static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
      -static char *sign_commit;
      +static char *sign_commit, *pathspec_from_file;
     @@ -87,14 +68,14 @@
      +			die(_("--pathspec-from-file is incompatible with --interactive/--patch"));
      +
      +		if (pathspec.nr)
     -+			die(_("--pathspec-from-file is incompatible with path arguments"));
     ++			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
      +
      +		parse_pathspec_file(&pathspec, 0,
      +				    PATHSPEC_PREFER_FULL,
     -+				    prefix, pathspec_from_file, pathspec_file_null);
     ++				    prefix, pathspec_from_file, pathspec_file_nul);
     ++	} else if (pathspec_file_nul) {
     ++		die(_("--pathspec-file-nul requires --pathspec-from-file"));
      +	}
     -+	else if (pathspec_file_null)
     -+		die(_("--pathspec-file-null requires --pathspec-from-file"));
      +
      +	if (!pathspec.nr && (also || (only && !amend && !allow_empty)))
      +		die(_("No paths with --include/--only does not make sense."));
     @@ -115,8 +96,8 @@
       		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
       		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
       		{ 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" },
     -+		OPT_FILENAME(0, "pathspec-from-file", &pathspec_from_file, N_("read pathspecs from file")),
     -+		OPT_BOOL(0, "pathspec-file-null", &pathspec_file_null, N_("with --pathspec-from-file, pathspecs are separated with NUL character")),
     ++		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
     ++		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
       		/* end commit contents options */
       
       		OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,
     @@ -134,23 +115,9 @@
      +
      +test_tick
      +
     -+cat > expect.a <<EOF
     -+A	fileA.t
     -+EOF
     -+
     -+cat > expect.ab <<EOF
     -+A	fileA.t
     -+A	fileB.t
     -+EOF
     -+
     -+cat > expect.bc <<EOF
     -+A	fileB.t
     -+A	fileC.t
     -+EOF
     -+
      +test_expect_success setup '
      +	test_commit file0 &&
     -+	checkpoint=$(git rev-parse --verify HEAD) &&
     ++	git tag checkpoint &&
      +	
      +	echo A >fileA.t &&
      +	echo B >fileB.t &&
     @@ -160,12 +127,12 @@
      +'
      +
      +restore_checkpoint () {
     -+	git reset --soft "$checkpoint"
     ++	git reset --soft checkpoint
      +}
      +
     -+verify_state () {
     ++verify_expect () {
      +	git diff-tree --no-commit-id --name-status -r HEAD >actual &&
     -+	test_cmp "$1" actual
     ++	test_cmp expect actual
      +}
      +
      +test_expect_success '--pathspec-from-file from stdin' '
     @@ -173,7 +140,10 @@
      +
      +	echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
      +
     -+	verify_state expect.a
     ++	cat >expect <<-\EOF &&
     ++	A	fileA.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success '--pathspec-from-file from file' '
     @@ -182,31 +152,58 @@
      +	echo fileA.t >list &&
      +	git commit --pathspec-from-file=list -m "Commit" &&
      +
     -+	verify_state expect.a
     ++	cat >expect <<-\EOF &&
     ++	A	fileA.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'NUL delimiters' '
      +	restore_checkpoint &&
      +
     -+	printf fileA.tQfileB.t | q_to_nul | git commit --pathspec-from-file=- --pathspec-file-null -m "Commit" &&
     ++	printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" &&
      +
     -+	verify_state expect.ab
     ++	cat >expect <<-\EOF &&
     ++	A	fileA.t
     ++	A	fileB.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'LF delimiters' '
      +	restore_checkpoint &&
      +
     ++	printf "fileA.t\nfileB.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
     ++
     ++	cat >expect <<-\EOF &&
     ++	A	fileA.t
     ++	A	fileB.t
     ++	EOF
     ++	verify_expect
     ++'
     ++
     ++test_expect_success 'no trailing delimiter' '
     ++	restore_checkpoint &&
     ++
      +	printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
      +
     -+	verify_state expect.ab
     ++	cat >expect <<-\EOF &&
     ++	A	fileA.t
     ++	A	fileB.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'CRLF delimiters' '
      +	restore_checkpoint &&
      +
     -+	printf "fileA.t\r\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
     ++	printf "fileA.t\r\nfileB.t\r\n" | git commit --pathspec-from-file=- -m "Commit" &&
      +
     -+	verify_state expect.ab
     ++	cat >expect <<-\EOF &&
     ++	A	fileA.t
     ++	A	fileB.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_expect_success 'quotes' '
     @@ -214,22 +211,29 @@
      +
      +	printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
      +
     -+	verify_state expect.a
     ++	cat >expect <<-\EOF &&
     ++	A	fileA.t
     ++	EOF
     ++	verify_expect expect
      +'
      +
     -+test_expect_success 'quotes not compatible with --pathspec-file-null' '
     ++test_expect_success 'quotes not compatible with --pathspec-file-nul' '
      +	restore_checkpoint &&
      +
      +	printf "\"file\\101.t\"" >list &&
     -+	test_must_fail git commit --pathspec-from-file=list --pathspec-file-null -m "Commit"
     ++	test_must_fail git commit --pathspec-from-file=list --pathspec-file-nul -m "Commit"
      +'
      +
      +test_expect_success 'only touches what was listed' '
      +	restore_checkpoint &&
      +
     -+	printf "fileB.t\nfileC.t" | git commit --pathspec-from-file=- -m "Commit" &&
     ++	printf "fileB.t\nfileC.t\n" | git commit --pathspec-from-file=- -m "Commit" &&
      +
     -+	verify_state expect.bc
     ++	cat >expect <<-\EOF &&
     ++	A	fileB.t
     ++	A	fileC.t
     ++	EOF
     ++	verify_expect
      +'
      +
      +test_done

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Synchronize it to git add, which has a pretty good description.

---
 Documentation/git-reset.txt | 20 +++++++++++---------
 builtin/reset.c             |  4 ++--
 2 files changed, 13 insertions(+), 11 deletions(-)

index 97e0544d9e..f2ccb1426c 100644
@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [-q] [<tree-ish>] [--] <paths>...
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION

@@ -18,24 +18,26 @@ In the first and second form, copy entries from `<tree-ish>` to the index.
 In the third form, set the current branch head (`HEAD`) to `<commit>`,
 optionally modifying index and working tree to match.
 The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
+The <pathspec> is used to limit the paths affected by the operation
+(see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).
 
-'git reset' [-q] [<tree-ish>] [--] <paths>...::
-	This form resets the index entries for all `<paths>` to their
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
+	This form resets the index entries for all `<pathspec>` to their
 	state at `<tree-ish>`.  (It does not affect the working tree or
 	the current branch.)
 +
-This means that `git reset <paths>` is the opposite of `git add
-<paths>`. This command is equivalent to
-`git restore [--source=<tree-ish>] --staged <paths>...`.
+This means that `git reset <pathspec>` is the opposite of `git add
+<pathspec>`. This command is equivalent to
+`git restore [--source=<tree-ish>] --staged <pathspec>...`.
 +
-After running `git reset <paths>` to update the index entry, you can
+After running `git reset <pathspec>` to update the index entry, you can
 use linkgit:git-restore[1] to check the contents out of the index to
 the working tree. Alternatively, using linkgit:git-restore[1]
 and specifying a commit with `--source`, you
 can copy the contents of a path out of a commit to the index and to the
 working tree in one go.
 
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]::
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]::
 	Interactively select hunks in the difference between the index
 	and `<tree-ish>` (defaults to `HEAD`).  The chosen hunks are applied
 	in reverse to the index.

index fdd572168b..9291c0fd72 100644
@@ -30,8 +30,8 @@
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
-	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
-	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
+	N_("git reset [-q] [<tree-ish>] [--] <pathspec>..."),
+	N_("git reset --patch [<tree-ish>] [--] [<pathspec>...]"),
 	NULL
 };
 

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Decisions taken for simplicity:
1) For now, --pathspec-from-file is declared incompatible with --interactive/--patch, even when <file> is not stdin. Such use case it not really expected. Also, it would require changes to interactive_add().
2) It is not allowed to pass pathspec in both args and file.

---
 Documentation/git-commit.txt    |  15 +++-
 builtin/commit.c                |  25 +++++-
 t/t7526-commit-pathspec-file.sh | 130 ++++++++++++++++++++++++++++++++
 3 files changed, 165 insertions(+), 5 deletions(-)
 create mode 100755 t/t7526-commit-pathspec-file.sh

index 915c212a0d..bbd53959bd 100644
@@ -13,7 +13,8 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
+	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
+	   [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -277,6 +278,18 @@ FROM UPSTREAM REBASE" section in linkgit:git-rebase[1].)
 	the last commit without committing changes that have
 	already been staged. If used together with `--allow-empty`
 	paths are also not required, and an empty commit will be created.
+	
+--pathspec-from-file=<file>::
+	Pathspec is passed in `<file>` instead of commandline args. If
+	`<file>` is exactly `-` then standard input is used. Pathspec
+	elements are separated by LF or CR/LF. Pathspec elements can be
+	quoted as explained for the configuration variable `core.quotePath`
+	(see linkgit:git-config[1]). See also `--pathspec-file-nul` and
+	global `--literal-pathspecs`.
+
+--pathspec-file-nul::
+	Only meaningful with `--pathspec-from-file`. Pathspec elements are
+	separated with NUL character and are not expected to be quoted.
 
 -u[<mode>]::
 --untracked-files[=<mode>]::

index e588bc6ad3..ed40729355 100644
@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
 static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
-static int no_post_rewrite, allow_empty_message;
+static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
-static char *sign_commit;
+static char *sign_commit, *pathspec_from_file;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
 
+	if (pathspec_from_file) {
+		if (interactive)
+			die(_("--pathspec-from-file is incompatible with --interactive/--patch"));
+
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
+
+		parse_pathspec_file(&pathspec, 0,
+				    PATHSPEC_PREFER_FULL,
+				    prefix, pathspec_from_file, pathspec_file_nul);
+	} else if (pathspec_file_nul) {
+		die(_("--pathspec-file-nul requires --pathspec-from-file"));
+	}
+
+	if (!pathspec.nr && (also || (only && !amend && !allow_empty)))
+		die(_("No paths with --include/--only does not make sense."));
+
 	if (read_cache_preload(&pathspec) < 0)
 		die(_("index file corrupt"));
 

@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
-	if (argc == 0 && (also || (only && !amend && !allow_empty)))
-		die(_("No paths with --include/--only does not make sense."));
 	cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);
 
 	handle_untracked_files_arg(s);

@@ -1535,6 +1550,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
 		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
 		{ 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" },
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
 		/* end commit contents options */
 
 		OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Synchronize it to git add, which has a pretty good description. This also better disambiguates <file>... header.

---
 Documentation/git-commit.txt | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

index afa7b75a23..915c212a0d 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<file>...]
+	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -345,12 +345,14 @@ changes to tracked files.
 \--::
 	Do not interpret any more arguments as options.
 
-<file>...::
-	When files are given on the command line, the command
-	commits the contents of the named files, without
-	recording the changes already staged.  The contents of
-	these files are also staged for the next commit on top
-	of what have been staged before.
+<pathspec>...::
+	When pathspec is given on the command line, commit the contents of
+	the files that match the pathspec without recording the changes
+	already added to the index. The contents of these files are also
+	staged for the next commit on top of what have been staged before.
++
+For more details about the <pathspec> syntax, see the 'pathspec' entry
+in linkgit:gitglossary[7].
 
 :git-commit: 1
 include::date-formats.txt[]

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Decisions taken for simplicity:
1) For now, --pathspec-from-file is declared incompatible with --patch, even when <file> is not stdin. Such use case it not really expected. Also, it is harder to support in git commit, so I decided to make it incompatible in all places.
2) It is not allowed to pass pathspec in both args and file.

---
 Documentation/git-reset.txt    |  19 +++-
 builtin/reset.c                |  21 ++++-
 t/t7107-reset-pathspec-file.sh | 155 +++++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+), 5 deletions(-)
 create mode 100755 t/t7107-reset-pathspec-file.sh

index f2ccb1426c..d2cd0af6a4 100644
@@ -9,20 +9,22 @@ SYNOPSIS
 --------
 [verse]
 'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-nul]] [<tree-ish>]
 'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION
 -----------
-In the first and second form, copy entries from `<tree-ish>` to the index.
-In the third form, set the current branch head (`HEAD`) to `<commit>`,
+In the first three forms, copy entries from `<tree-ish>` to the index.
+In the last form, set the current branch head (`HEAD`) to `<commit>`,
 optionally modifying index and working tree to match.
 The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
 The <pathspec> is used to limit the paths affected by the operation
 (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).
 
 'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
-	This form resets the index entries for all `<pathspec>` to their
+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-nul]] [<tree-ish>]::
+	These forms reset the index entries matching pathspec to their
 	state at `<tree-ish>`.  (It does not affect the working tree or
 	the current branch.)
 +

@@ -103,6 +105,17 @@ OPTIONS
 	`reset.quiet` config option. `--quiet` and `--no-quiet` will
 	override the default behavior.
 
+--pathspec-from-file=<file>::
+	Pathspec is passed in `<file>` instead of commandline args. If
+	`<file>` is exactly `-` then standard input is used. Pathspec
+	elements are separated by LF or CR/LF. Pathspec elements can be
+	quoted as explained for the configuration variable `core.quotePath`
+	(see linkgit:git-config[1]). See also `--pathspec-file-nul` and
+	global `--literal-pathspecs`.
+
+--pathspec-file-nul::
+	Only meaningful with `--pathspec-from-file`. Pathspec elements are
+	separated with NUL character and are not expected to be quoted.
 
 EXAMPLES
 --------

index 9291c0fd72..246bf9d737 100644
@@ -31,6 +31,7 @@
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 	N_("git reset [-q] [<tree-ish>] [--] <pathspec>..."),
+	N_("git reset [-q] [--pathspec-from-file [--pathspec-file-nul]] [<tree-ish>]"),
 	N_("git reset --patch [<tree-ish>] [--] [<pathspec>...]"),
 	NULL
 };

@@ -284,8 +285,8 @@ static int git_reset_config(const char *var, const char *value, void *cb)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0, unborn;
-	const char *rev;
+	int patch_mode = 0, pathspec_file_nul = 0, unborn;
+	const char *rev, *pathspec_from_file = NULL;
 	struct object_id oid;
 	struct pathspec pathspec;
 	int intent_to_add = 0;

@@ -306,6 +307,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
 		OPT_END()
 	};
 

@@ -316,6 +319,20 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	if (pathspec_from_file) {
+		if (patch_mode)
+			die(_("--pathspec-from-file is incompatible with --patch"));
+
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
+
+		parse_pathspec_file(&pathspec, 0,
+				    PATHSPEC_PREFER_FULL,
+				    prefix, pathspec_from_file, pathspec_file_nul);
+	} else if (pathspec_file_nul) {
+		die(_("--pathspec-file-nul requires --pathspec-from-file"));
+	}
+
 	unborn = !strcmp(rev, "HEAD") && get_oid("HEAD", &oid);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */

gitgitgadget

I think I have implemented all suggestions in PatchV2. Thanks!

I think I have implemented most suggestions in PatchV2. Thanks!

I have improved commit message to clarify.

I decided to stay with writing suggested by Junio.

I think it's good. Maybe you were confused by patch formatting?

I think it's good again :)

Already agreed that test is good, just a bit non-obvious.

I decided not to repeat '--pathspec-from-file' in every test, because test package already has it in description. Is that good?

I think I have implemented most suggestions in PatchV2. Thanks!

I decided to move the general comment to base commit where options are introduced and not repeat it where options are supported.

The code looks a bit too entangled to support that without making it worse. The biggest concern I have is that parse_and_validate_options() will populate pathspec and some other function will need to remember to clean it up. I like it better when pathspec is handled in one place.

I agree that things are not perfect, but this seems to be a consequence of other existing problems. For example, I would have expected a structure instead of a handful of global variables. That would have solved many problems. However, I didn't have the bravery to dive into this refactoring.

I think I have implemented most suggestions in PatchV2. Thanks!

I am a bit confused, is it correct that you don't expect me to change my patches?

I think I have implemented most suggestions in PatchV2. Thanks!

Let's think that this merely reinforces the first point :)

I didn't want to run into a situation where '-z' suddenly applies to multiple things (for example, if later it's decided to allow --patch with --pathspec-from-file).

Regarding UI, I think that (insert our UI name here) will not use '--pathspec-from-file' unless there are really many files. When it does happen, user will probably have an easier time understanding '--pathspec-file-nul' then '-z'.

I changed that. However, git grep pathspecs is quite interesting. My own taste says that "pathspecs" is more expected then "pathspec".

Yes, NUL delimiter allows to avoid escaping.

Tests overwhelmingly prefer escaped form (\EOF) instead, so I used that.

Kind of intended. I changed tests to always use trailing delimiters, and also added a new dedicated test without trailing delimiter.

I think I have implemented most suggestions in PatchV2. Thanks!

Commit is a well known term, dating from ancient times like CVS or even older.

Pathspec, however, is something new. When I pretend to be someone new to git, I see it this way:
1) Let's read "git commit" documentation
2) Where on this commandline do I put my filename?!

So yes, I would repeat it in every location that could be popular for new users.

I do not know if removal of the only-half-correct "This is the opposite of add" should be part of this change, or it should be a separate fix. The half-wrong sentene was not introduced by this patch, so leaving it as-is is OK. It just leaves another thing for us to think about later.

Thanks.

That's more or less irrelevant.

I am reacting to this from your change that you omitted quoting in your reply:

Compared to CVS, everything in Git may be new, but that was a news in 2010, not this year.

My feeling is that this problem is separate from the topic I'm working on. I only touched docs to synchronize <pathspec>, so that I can copy&paste new option description in next commit without tailoring it to local speech. Also, I must admit that upon reading your explanation, I felt that I lack the experience to update the writing properly.

In the perfect world, I would expect _every_ 'pathspec' word in text to be an HTML-style link to a dedicated article, not just a paragraph in glossary.

MSDN is in general a good example [1]: there, it's easy to read only a small portion of article, ignoring anything you're not interested in, and still have all links at hand.

Regarding dedicated page: the content of 'pathspec' in glossary is already long enough for a page, and it could benefit from additional examples. Also, having a dedicated page makes linking easier, so that user doesn't have to scroll glossary.

Regarding links: I don't really understand what git's doc format allows. Is it just pure text in worst (or even average) case?

If it's usually something with clickable links, it could be worth to just insert links everywhere.

If it's usually plaintext, then "see the 'pathspec' entry in linkgit:gitglossary[7]" is a bit too verbose to repeat on every occasion. Still, I'd like to see links everywhere. One big reason is to let reader know that the explanation actually exists!

A compromise solution is to give every article header like this:

This article uses the following terms which are explained in linkgit:gitglossary[7]:
* pathspec
* tree-ish
* refspec

If it's close to top of article, then chances are that everyone will notice it. Also, it will not require extra verbosity in plaintext.

I now think that my arguments apply to <pathspec> AND <commit> in the same way. Indeed a user can't know complex <commit> variants until he/she reads it in git docs.

----

[1] https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea

Yeah, I do think it is cleaner to leave it as a separate issue.

Thanks.

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

Good.

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

Changes from V2:

1) Some polishing in documentation 2) = NULL removed from parse_pathspec_file()

Alexandr Miloslavskiy (6):
parse-options.h: add new options --pathspec-from-file,

    `--pathspec-file-nul`

pathspec: add new function to parse file
doc: reset: synchronize <pathspec> description
reset: support the --pathspec-from-file option
doc: commit: synchronize <pathspec> description
commit: support the --pathspec-from-file option

 Documentation/git-commit.txt    |  29 ++++--
 Documentation/git-reset.txt     |  48 +++++++---
 builtin/commit.c                |  25 +++++-
 builtin/reset.c                 |  25 +++++-
 parse-options.h                 |   2 +
 pathspec.c                      |  38 ++++++++
 pathspec.h                      |  10 +++
 t/t7107-reset-pathspec-file.sh  | 155 ++++++++++++++++++++++++++++++++
 t/t7526-commit-pathspec-file.sh | 130 +++++++++++++++++++++++++++
 9 files changed, 434 insertions(+), 28 deletions(-)
 create mode 100755 t/t7107-reset-pathspec-file.sh
 create mode 100755 t/t7526-commit-pathspec-file.sh

base-commit: da72936f544fec5a335e66432610e4cef4430991
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/445

Range-diff vs v2:

 1:  2dfaccf0d5 = 1:  19b80326ea parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`
 2:  96697ba072 ! 2:  55a7c6ec3c pathspec: add new function to parse file
     @@ -45,7 +45,7 @@
      +						       strbuf_getline;
      +	struct strbuf buf = STRBUF_INIT;
      +	struct strbuf unquoted = STRBUF_INIT;
     -+	FILE *in = NULL;
     ++	FILE *in;
      +
      +	if (!strcmp(file, "-"))
      +		in = stdin;

3: f961a5155a ! 3: d9f32e523c doc: reset: unify <pathspec> description
@@ -1,8 +1,8 @@
Author: Alexandr Miloslavskiy <[email protected]>

- doc: reset: unify <pathspec> description
+ doc: reset: synchronize <pathspec> description

- Synchronize it to git add, which has a pretty good description.
+ git add shows an example of good writing, follow it.

@@ -21,18 +21,17 @@

DESCRIPTION
@@
- In the third form, set the current branch head (HEAD) to <commit>, optionally modifying index and working tree to match.
The <tree-ish>/<commit> defaults to HEAD in all forms.
-+The <pathspec> is used to limit the paths affected by the operation -+(see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).

-'git reset' [-q] [<tree-ish>] [--] <paths>...::
- This form resets the index entries for all <paths> to their
+- state at <tree-ish>. (It does not affect the working tree or
+- the current branch.)
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
-+ This form resets the index entries for all <pathspec> to their
- state at <tree-ish>. (It does not affect the working tree or
- the current branch.)
++ This form resets the index entries for all paths that match the
++ <pathspec> to their state at <tree-ish>. (It does not affect
++ the working tree or the current branch.)
+
-This means that git reset <paths> is the opposite of git add -<paths>. This command is equivalent to
@@ -54,6 +53,20 @@
Interactively select hunks in the difference between the index and <tree-ish> (defaults to HEAD). The chosen hunks are applied in reverse to the index.
+@@
+ reset.quiet config option. --quiet and --no-quiet will
+ override the default behavior.
+
++\--::
++ Do not interpret any more arguments as options.
++
++<pathspec>...::
++ Limits the paths affected by the operation.
+++
++For more details, see the 'pathspec' entry in linkgit:gitglossary[7]. +
+ EXAMPLES
+ --------

diff --git a/builtin/reset.c b/builtin/reset.c
--- a/builtin/reset.c
4: d72d4f16b5 ! 4: 8a10ff881b reset: support the --pathspec-from-file option
@@ -31,15 +31,13 @@
+In the last form, set the current branch head (HEAD) to <commit>,
optionally modifying index and working tree to match.
The <tree-ish>/<commit> defaults to HEAD in all forms.
- The <pathspec> is used to limit the paths affected by the operation
- (see the entry for 'pathspec' in linkgit:gitglossary[7] for more details).

'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
-- This form resets the index entries for all <pathspec> to their
+- This form resets the index entries for all paths that match the
+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-nul]] [<tree-ish>]::
-+ These forms reset the index entries matching pathspec to their
- state at <tree-ish>. (It does not affect the working tree or
- the current branch.)
++ These forms reset the index entries for all paths that match the
+ <pathspec> to their state at <tree-ish>. (It does not affect
+ the working tree or the current branch.)
+
@@
reset.quiet config option. --quiet and --no-quiet will
@@ -55,10 +53,12 @@
+
+--pathspec-file-nul::
+ Only meaningful with --pathspec-from-file. Pathspec elements are
-+ separated with NUL character and are not expected to be quoted.
++ separated with NUL character and all other characters are taken
++ literally (including newlines and quotes).
++
+ \--::
+ Do not interpret any more arguments as options.

- EXAMPLES
- --------

diff --git a/builtin/reset.c b/builtin/reset.c
--- a/builtin/reset.c
@@ -241,7 +241,7 @@
+ printf "\"file\\101.t\"" >list &&
+ # Note: "git reset" has not yet learned to fail on wrong pathspecs
+ git reset --pathspec-from-file=list --pathspec-file-nul &&
-+
++
+ cat >expect <<-\EOF &&
+ D fileA.t
+ EOF
5: 20c4495fd3 ! 5: 0b79797e77 doc: commit: unify <pathspec> description
@@ -1,8 +1,8 @@
Author: Alexandr Miloslavskiy <[email protected]>

- doc: commit: unify <pathspec> description
+ doc: commit: synchronize <pathspec> description

- Synchronize it to git add, which has a pretty good description.
+ git add shows an example of good writing, follow it.
This also better disambiguates <file>... header.

@@ -35,8 +35,7 @@
+ already added to the index. The contents of these files are also
+ staged for the next commit on top of what have been staged before.
++
-+For more details about the <pathspec> syntax, see the 'pathspec' entry -+in linkgit:gitglossary[7].
++For more details, see the 'pathspec' entry in linkgit:gitglossary[7].

:git-commit: 1
include::date-formats.txt[]
6: cb5fc9b14d ! 6: 7e48212002 commit: support the --pathspec-from-file option

     @@ -25,10 +25,9 @@
       DESCRIPTION
       -----------
      @@
     - 	the last commit without committing changes that have
       	already been staged. If used together with `--allow-empty`
       	paths are also not required, and an empty commit will be created.
     -+	
     + 
      +--pathspec-from-file=<file>::
      +	Pathspec is passed in `<file>` instead of commandline args. If
      +	`<file>` is exactly `-` then standard input is used. Pathspec
     @@ -39,10 +38,12 @@
      +
      +--pathspec-file-nul::
      +	Only meaningful with `--pathspec-from-file`. Pathspec elements are
     -+	separated with NUL character and are not expected to be quoted.
     - 
     ++	separated with NUL character and all other characters are taken
     ++	literally (including newlines and quotes).
     ++
       -u[<mode>]::
       --untracked-files[=<mode>]::
     + 	Show untracked files.
      
       diff --git a/builtin/commit.c b/builtin/commit.c
       --- a/builtin/commit.c
     @@ -118,7 +119,7 @@
      +test_expect_success setup '
      +	test_commit file0 &&
      +	git tag checkpoint &&
     -+	
     ++
      +	echo A >fileA.t &&
      +	echo B >fileB.t &&
      +	echo C >fileC.t &&

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

git add shows an example of good writing, follow it.

---
 Documentation/git-reset.txt | 29 ++++++++++++++++++-----------
 builtin/reset.c             |  4 ++--
 2 files changed, 20 insertions(+), 13 deletions(-)

index 97e0544d9e..d517a43e73 100644
@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [-q] [<tree-ish>] [--] <paths>...
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION

@@ -19,23 +19,23 @@ In the third form, set the current branch head (`HEAD`) to `<commit>`,
 optionally modifying index and working tree to match.
 The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
 
-'git reset' [-q] [<tree-ish>] [--] <paths>...::
-	This form resets the index entries for all `<paths>` to their
-	state at `<tree-ish>`.  (It does not affect the working tree or
-	the current branch.)
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
+	This form resets the index entries for all paths that match the
+	`<pathspec>` to their state at `<tree-ish>`.  (It does not affect
+	the working tree or the current branch.)
 +
-This means that `git reset <paths>` is the opposite of `git add
-<paths>`. This command is equivalent to
-`git restore [--source=<tree-ish>] --staged <paths>...`.
+This means that `git reset <pathspec>` is the opposite of `git add
+<pathspec>`. This command is equivalent to
+`git restore [--source=<tree-ish>] --staged <pathspec>...`.
 +
-After running `git reset <paths>` to update the index entry, you can
+After running `git reset <pathspec>` to update the index entry, you can
 use linkgit:git-restore[1] to check the contents out of the index to
 the working tree. Alternatively, using linkgit:git-restore[1]
 and specifying a commit with `--source`, you
 can copy the contents of a path out of a commit to the index and to the
 working tree in one go.
 
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]::
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]::
 	Interactively select hunks in the difference between the index
 	and `<tree-ish>` (defaults to `HEAD`).  The chosen hunks are applied
 	in reverse to the index.

@@ -101,6 +101,13 @@ OPTIONS
 	`reset.quiet` config option. `--quiet` and `--no-quiet` will
 	override the default behavior.
 
+\--::
+	Do not interpret any more arguments as options.
+
+<pathspec>...::
+	Limits the paths affected by the operation.
++
+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 EXAMPLES
 --------

index fdd572168b..9291c0fd72 100644
@@ -30,8 +30,8 @@
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
-	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
-	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
+	N_("git reset [-q] [<tree-ish>] [--] <pathspec>..."),
+	N_("git reset --patch [<tree-ish>] [--] [<pathspec>...]"),
 	NULL
 };
 

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

This will be used to support the new option '--pathspec-from-file' in git add, git-commit, git reset etc.

Note also that we specifically handle CR/LF line endings to support Windows better.

To simplify code, file is first parsed into argv_array. This allows to avoid refactoring parse_pathspec().

I considered adding nul_term_line to flags instead, but decided that it doesn't fit there.

The new code is mostly taken from cmd_update_index() and split_mail_conv().

---
 pathspec.c | 38 ++++++++++++++++++++++++++++++++++++++
 pathspec.h | 10 ++++++++++
 2 files changed, 48 insertions(+)

index 12c2b322b3..128f27fcb7 100644
@@ -3,6 +3,8 @@
 #include "dir.h"
 #include "pathspec.h"
 #include "attr.h"
+#include "argv-array.h"
+#include "quote.h"
 
 /*
  * Finds which of the given pathspecs match items in the index.

@@ -613,6 +615,42 @@ void parse_pathspec(struct pathspec *pathspec,
 	}
 }
 
+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,
+			 unsigned flags, const char *prefix,
+			 const char *file, int nul_term_line)
+{
+	struct argv_array parsed_file = ARGV_ARRAY_INIT;
+	strbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :
+						       strbuf_getline;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf unquoted = STRBUF_INIT;
+	FILE *in;
+
+	if (!strcmp(file, "-"))
+		in = stdin;
+	else
+		in = xfopen(file, "r");
+
+	while (getline_fn(&buf, in) != EOF) {
+		if (!nul_term_line && buf.buf[0] == '"') {
+			strbuf_reset(&unquoted);
+			if (unquote_c_style(&unquoted, buf.buf, NULL))
+				die(_("line is badly quoted: %s"), buf.buf);
+			strbuf_swap(&buf, &unquoted);
+		}
+		argv_array_push(&parsed_file, buf.buf);
+		strbuf_reset(&buf);
+	}
+
+	strbuf_release(&unquoted);
+	strbuf_release(&buf);
+	if (in != stdin)
+		fclose(in);
+
+	parse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);
+	argv_array_clear(&parsed_file);
+}
+
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
 {
 	int i, j;

index 1c18a2c90c..a27dc81ba2 100644
@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,
 		    unsigned flags,
 		    const char *prefix,
 		    const char **args);
+/*
+ * Same as parse_pathspec() but uses file as input.
+ * When 'file' is exactly "-" it uses 'stdin' instead.
+ */
+void parse_pathspec_file(struct pathspec *pathspec,
+			 unsigned magic_mask,
+			 unsigned flags,
+			 const char *prefix,
+			 const char *file,
+			 int nul_term_line);
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
 void clear_pathspec(struct pathspec *);
 

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

git add shows an example of good writing, follow it. This also better disambiguates <file>... header.

---
 Documentation/git-commit.txt | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

index afa7b75a23..a0c44978ee 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<file>...]
+	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -345,12 +345,13 @@ changes to tracked files.
 \--::
 	Do not interpret any more arguments as options.
 
-<file>...::
-	When files are given on the command line, the command
-	commits the contents of the named files, without
-	recording the changes already staged.  The contents of
-	these files are also staged for the next commit on top
-	of what have been staged before.
+<pathspec>...::
+	When pathspec is given on the command line, commit the contents of
+	the files that match the pathspec without recording the changes
+	already added to the index. The contents of these files are also
+	staged for the next commit on top of what have been staged before.
++
+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 :git-commit: 1
 include::date-formats.txt[]

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Decisions taken for simplicity:
1) For now, --pathspec-from-file is declared incompatible with --patch, even when <file> is not stdin. Such use case it not really expected. Also, it is harder to support in git commit, so I decided to make it incompatible in all places.
2) It is not allowed to pass pathspec in both args and file.

---
 Documentation/git-reset.txt    |  21 ++++-
 builtin/reset.c                |  21 ++++-
 t/t7107-reset-pathspec-file.sh | 155 +++++++++++++++++++++++++++++++++
 3 files changed, 192 insertions(+), 5 deletions(-)
 create mode 100755 t/t7107-reset-pathspec-file.sh

index d517a43e73..932080c55d 100644
@@ -9,18 +9,20 @@ SYNOPSIS
 --------
 [verse]
 'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-nul]] [<tree-ish>]
 'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION
 -----------
-In the first and second form, copy entries from `<tree-ish>` to the index.
-In the third form, set the current branch head (`HEAD`) to `<commit>`,
+In the first three forms, copy entries from `<tree-ish>` to the index.
+In the last form, set the current branch head (`HEAD`) to `<commit>`,
 optionally modifying index and working tree to match.
 The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
 
 'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
-	This form resets the index entries for all paths that match the
+'git reset' [-q] [--pathspec-from-file=<file> [--pathspec-file-nul]] [<tree-ish>]::
+	These forms reset the index entries for all paths that match the
 	`<pathspec>` to their state at `<tree-ish>`.  (It does not affect
 	the working tree or the current branch.)
 +

@@ -101,6 +103,19 @@ OPTIONS
 	`reset.quiet` config option. `--quiet` and `--no-quiet` will
 	override the default behavior.
 
+--pathspec-from-file=<file>::
+	Pathspec is passed in `<file>` instead of commandline args. If
+	`<file>` is exactly `-` then standard input is used. Pathspec
+	elements are separated by LF or CR/LF. Pathspec elements can be
+	quoted as explained for the configuration variable `core.quotePath`
+	(see linkgit:git-config[1]). See also `--pathspec-file-nul` and
+	global `--literal-pathspecs`.
+
+--pathspec-file-nul::
+	Only meaningful with `--pathspec-from-file`. Pathspec elements are
+	separated with NUL character and all other characters are taken
+	literally (including newlines and quotes).
+
 \--::
 	Do not interpret any more arguments as options.
 

index 9291c0fd72..246bf9d737 100644
@@ -31,6 +31,7 @@
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
 	N_("git reset [-q] [<tree-ish>] [--] <pathspec>..."),
+	N_("git reset [-q] [--pathspec-from-file [--pathspec-file-nul]] [<tree-ish>]"),
 	N_("git reset --patch [<tree-ish>] [--] [<pathspec>...]"),
 	NULL
 };

@@ -284,8 +285,8 @@ static int git_reset_config(const char *var, const char *value, void *cb)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
 	int reset_type = NONE, update_ref_status = 0, quiet = 0;
-	int patch_mode = 0, unborn;
-	const char *rev;
+	int patch_mode = 0, pathspec_file_nul = 0, unborn;
+	const char *rev, *pathspec_from_file = NULL;
 	struct object_id oid;
 	struct pathspec pathspec;
 	int intent_to_add = 0;

@@ -306,6 +307,8 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")),
 		OPT_BOOL('N', "intent-to-add", &intent_to_add,
 				N_("record only the fact that removed paths will be added later")),
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
 		OPT_END()
 	};
 

@@ -316,6 +319,20 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 						PARSE_OPT_KEEP_DASHDASH);
 	parse_args(&pathspec, argv, prefix, patch_mode, &rev);
 
+	if (pathspec_from_file) {
+		if (patch_mode)
+			die(_("--pathspec-from-file is incompatible with --patch"));
+
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
+
+		parse_pathspec_file(&pathspec, 0,
+				    PATHSPEC_PREFER_FULL,
+				    prefix, pathspec_from_file, pathspec_file_nul);
+	} else if (pathspec_file_nul) {
+		die(_("--pathspec-file-nul requires --pathspec-from-file"));
+	}
+
 	unborn = !strcmp(rev, "HEAD") && get_oid("HEAD", &oid);
 	if (unborn) {
 		/* reset on unborn branch: treat as reset to empty tree */

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Support for various porcelain commands will arrive via additional patches.

--pathspec-from-file solves the problem of commandline length limit for UIs built on top of git. Plumbing commands are not always a good fit, for two major reasons:
1) Some UIs show executed commands to user. In this case, porcelain commands are expected. One reason for that is letting user learn git commands by clicking UI buttons. The other reason is letting user study the history of commands in case of any unexpected results. Both of these will lose most of their value if UI uses combinations of arcane plumbing commands.
2) Some UIs have started and grown with porcelain commands. Replacing existing logic with plumbing commands could be cumbersome and prone to various new problems.

--pathspec-from-file will behave very close to pathspec passed in commandline args, so that switching from one to another is simple.

--pathspec-from-file will read either a specified file or stdin (when file is exactly "-"). Reading from file is a good way to avoid competing for stdin, and also gives some extra flexibility.

--pathspec-file-nul switch mirrors -z already used in various places. Some porcelain commands, such as git commit, already use -z, therefore it needed a new unambiguous name.

New options do not have shorthands to avoid shorthand conflicts. It is not expected that they will be typed in console.

---
 parse-options.h | 2 ++
 1 file changed, 2 insertions(+)

index 38a33a087e..c6cc01e715 100644
@@ -330,5 +330,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 #define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 #define OPT_CLEANUP(v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))
+#define OPT_PATHSPEC_FROM_FILE(v) OPT_FILENAME(0, "pathspec-from-file", v, N_("read pathspec from file"))
+#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"))
 
 #endif

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Decisions taken for simplicity:
1) For now, --pathspec-from-file is declared incompatible with --interactive/--patch, even when <file> is not stdin. Such use case it not really expected. Also, it would require changes to interactive_add().
2) It is not allowed to pass pathspec in both args and file.

---
 Documentation/git-commit.txt    |  16 +++-
 builtin/commit.c                |  25 +++++-
 t/t7526-commit-pathspec-file.sh | 130 ++++++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+), 5 deletions(-)
 create mode 100755 t/t7526-commit-pathspec-file.sh

index a0c44978ee..ced5a9beab 100644
@@ -13,7 +13,8 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
+	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
+	   [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -278,6 +279,19 @@ FROM UPSTREAM REBASE" section in linkgit:git-rebase[1].)
 	already been staged. If used together with `--allow-empty`
 	paths are also not required, and an empty commit will be created.
 
+--pathspec-from-file=<file>::
+	Pathspec is passed in `<file>` instead of commandline args. If
+	`<file>` is exactly `-` then standard input is used. Pathspec
+	elements are separated by LF or CR/LF. Pathspec elements can be
+	quoted as explained for the configuration variable `core.quotePath`
+	(see linkgit:git-config[1]). See also `--pathspec-file-nul` and
+	global `--literal-pathspecs`.
+
+--pathspec-file-nul::
+	Only meaningful with `--pathspec-from-file`. Pathspec elements are
+	separated with NUL character and all other characters are taken
+	literally (including newlines and quotes).
+
 -u[<mode>]::
 --untracked-files[=<mode>]::
 	Show untracked files.

index e588bc6ad3..ed40729355 100644
@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
 static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
-static int no_post_rewrite, allow_empty_message;
+static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
-static char *sign_commit;
+static char *sign_commit, *pathspec_from_file;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
 
+	if (pathspec_from_file) {
+		if (interactive)
+			die(_("--pathspec-from-file is incompatible with --interactive/--patch"));
+
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
+
+		parse_pathspec_file(&pathspec, 0,
+				    PATHSPEC_PREFER_FULL,
+				    prefix, pathspec_from_file, pathspec_file_nul);
+	} else if (pathspec_file_nul) {
+		die(_("--pathspec-file-nul requires --pathspec-from-file"));
+	}
+
+	if (!pathspec.nr && (also || (only && !amend && !allow_empty)))
+		die(_("No paths with --include/--only does not make sense."));
+
 	if (read_cache_preload(&pathspec) < 0)
 		die(_("index file corrupt"));
 

@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
-	if (argc == 0 && (also || (only && !amend && !allow_empty)))
-		die(_("No paths with --include/--only does not make sense."));
 	cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);
 
 	handle_untracked_files_arg(s);

@@ -1535,6 +1550,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
 		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
 		{ 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" },
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
 		/* end commit contents options */
 
 		OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,

gitgitgadget

Changed in V3.

Yes, I wasn't paying enough attention.
Changed in V3, thanks!

I shortened it even more in V3, I think it's better now?

I think that a reference to core.quotePath could be confusing here, because it doesn't really affect the option. I have reworded it in V3, please see if you like it.

Thanks for pointing out!

This has shown me that I didn't synchronize docs enough. I have studied docs for other commands and found out that most of them list <pathspec> in a separate paragraph under options.

I find it very reasonable, because that's where I would normally expect it as a reader, together with other options.

So I adjusted 'git-reset' docs to also list <pathspec> under options.

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>

Nicely done. I think this is now ready for 'next'.

Thanks.

Wonderful, thanks for your assistance!

I will continue with patches for other commands, then.

Changes from V3:
================

The branch was rebased onto latest master. These patches remain unchanged since they were accepted in V3: parse-options.h: add new options --pathspec-from-file, --pathspec-file-nulpathspec: add new function to parse file doc: reset: synchronize description reset: support the --pathspec-from-file option doc: commit: synchronize description commit:
support the --pathspec-from-file option These patches are new, extending support to more git commands: cmd_add: prepare for next patch add: support the --pathspec-from-file option doc: checkout: remove duplicate synopsis
doc: checkout: fix broken text reference doc: checkout: synchronize description doc: restore: synchronize description checkout, restore: support the --pathspec-from-file option

Alexandr Miloslavskiy (13):
parse-options.h: add new options --pathspec-from-file,

    `--pathspec-file-nul`

pathspec: add new function to parse file
doc: reset: synchronize <pathspec> description
reset: support the --pathspec-from-file option
doc: commit: synchronize <pathspec> description
commit: support the --pathspec-from-file option
cmd_add: prepare for next patch
add: support the --pathspec-from-file option
doc: checkout: remove duplicate synopsis
doc: checkout: fix broken text reference
doc: checkout: synchronize <pathspec> description
doc: restore: synchronize <pathspec> description checkout, restore: support the --pathspec-from-file option

 Documentation/git-add.txt         |  16 ++-
 Documentation/git-checkout.txt    |  50 +++++++---
 Documentation/git-commit.txt      |  29 ++++--
 Documentation/git-reset.txt       |  48 ++++++---
 Documentation/git-restore.txt     |  26 ++++-
 builtin/add.c                     |  60 ++++++++----
 builtin/checkout.c                |  31 +++++-
 builtin/commit.c                  |  25 ++++-
 builtin/reset.c                   |  25 ++++-
 parse-options.h                   |   2 +
 pathspec.c                        |  38 ++++++++
 pathspec.h                        |  10 ++
 t/t2026-checkout-pathspec-file.sh | 139 +++++++++++++++++++++++++++
 t/t2072-restore-pathspec-file.sh  | 139 +++++++++++++++++++++++++++
 t/t3704-add-pathspec-file.sh      | 127 ++++++++++++++++++++++++
 t/t7107-reset-pathspec-file.sh    | 155 ++++++++++++++++++++++++++++++
 t/t7526-commit-pathspec-file.sh   | 130 +++++++++++++++++++++++++
 t/t9902-completion.sh             |   2 +
 18 files changed, 982 insertions(+), 70 deletions(-)
 create mode 100755 t/t2026-checkout-pathspec-file.sh
 create mode 100755 t/t2072-restore-pathspec-file.sh
 create mode 100755 t/t3704-add-pathspec-file.sh
 create mode 100755 t/t7107-reset-pathspec-file.sh
 create mode 100755 t/t7526-commit-pathspec-file.sh

base-commit: 228f53135a4a41a37b6be8e4d6e2b6153db4a8ed
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-445%2FSyntevoAlex%2F%230207_pathspec_from_file-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-445/SyntevoAlex/#0207_pathspec_from_file-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/445

Range-diff vs v3:

  1:  19b80326ea =  1:  cca5aee392 parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`
  2:  55a7c6ec3c =  2:  fea64dfbf9 pathspec: add new function to parse file
  3:  d9f32e523c =  3:  1182ba3953 doc: reset: synchronize <pathspec> description
  4:  8a10ff881b =  4:  cea470fc91 reset: support the `--pathspec-from-file` option
  5:  0b79797e77 =  5:  0e1ac7e8a7 doc: commit: synchronize <pathspec> description
  6:  7e48212002 =  6:  c877866c13 commit: support the --pathspec-from-file option
  -:  ---------- >  7:  a97910cb55 cmd_add: prepare for next patch
  -:  ---------- >  8:  9a62da3470 add: support the --pathspec-from-file option
  -:  ---------- >  9:  5e449c8d29 doc: checkout: remove duplicate synopsis
  -:  ---------- > 10:  a498dda97b doc: checkout: fix broken text reference
  -:  ---------- > 11:  fd166755aa doc: checkout: synchronize <pathspec> description
  -:  ---------- > 12:  9e37a740e6 doc: restore: synchronize <pathspec> description
  -:  ---------- > 13:  c4dd4eaf13 checkout, restore: support the --pathspec-from-file option

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Support for various porcelain commands will arrive via additional patches.

--pathspec-from-file solves the problem of commandline length limit for UIs built on top of git. Plumbing commands are not always a good fit, for two major reasons:
1) Some UIs show executed commands to user. In this case, porcelain commands are expected. One reason for that is letting user learn git commands by clicking UI buttons. The other reason is letting user study the history of commands in case of any unexpected results. Both of these will lose most of their value if UI uses combinations of arcane plumbing commands.
2) Some UIs have started and grown with porcelain commands. Replacing existing logic with plumbing commands could be cumbersome and prone to various new problems.

--pathspec-from-file will behave very close to pathspec passed in commandline args, so that switching from one to another is simple.

--pathspec-from-file will read either a specified file or stdin (when file is exactly "-"). Reading from file is a good way to avoid competing for stdin, and also gives some extra flexibility.

--pathspec-file-nul switch mirrors -z already used in various places. Some porcelain commands, such as git commit, already use -z, therefore it needed a new unambiguous name.

New options do not have shorthands to avoid shorthand conflicts. It is not expected that they will be typed in console.

---
 parse-options.h | 2 ++
 1 file changed, 2 insertions(+)

index 38a33a087e..c6cc01e715 100644
@@ -330,5 +330,7 @@ int parse_opt_passthru_argv(const struct option *, const char *, int);
 #define OPT_WITH(v, h) _OPT_CONTAINS_OR_WITH("with", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 #define OPT_WITHOUT(v, h) _OPT_CONTAINS_OR_WITH("without", v, h, PARSE_OPT_HIDDEN | PARSE_OPT_NONEG)
 #define OPT_CLEANUP(v) OPT_STRING(0, "cleanup", v, N_("mode"), N_("how to strip spaces and #comments from message"))
+#define OPT_PATHSPEC_FROM_FILE(v) OPT_FILENAME(0, "pathspec-from-file", v, N_("read pathspec from file"))
+#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"))
 
 #endif

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

git add shows an example of good writing, follow it.

---
 Documentation/git-reset.txt | 29 ++++++++++++++++++-----------
 builtin/reset.c             |  4 ++--
 2 files changed, 20 insertions(+), 13 deletions(-)

index 97e0544d9e..d517a43e73 100644
@@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state
 SYNOPSIS
 --------
 [verse]
-'git reset' [-q] [<tree-ish>] [--] <paths>...
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]
 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [<commit>]
 
 DESCRIPTION

@@ -19,23 +19,23 @@ In the third form, set the current branch head (`HEAD`) to `<commit>`,
 optionally modifying index and working tree to match.
 The `<tree-ish>`/`<commit>` defaults to `HEAD` in all forms.
 
-'git reset' [-q] [<tree-ish>] [--] <paths>...::
-	This form resets the index entries for all `<paths>` to their
-	state at `<tree-ish>`.  (It does not affect the working tree or
-	the current branch.)
+'git reset' [-q] [<tree-ish>] [--] <pathspec>...::
+	This form resets the index entries for all paths that match the
+	`<pathspec>` to their state at `<tree-ish>`.  (It does not affect
+	the working tree or the current branch.)
 +
-This means that `git reset <paths>` is the opposite of `git add
-<paths>`. This command is equivalent to
-`git restore [--source=<tree-ish>] --staged <paths>...`.
+This means that `git reset <pathspec>` is the opposite of `git add
+<pathspec>`. This command is equivalent to
+`git restore [--source=<tree-ish>] --staged <pathspec>...`.
 +
-After running `git reset <paths>` to update the index entry, you can
+After running `git reset <pathspec>` to update the index entry, you can
 use linkgit:git-restore[1] to check the contents out of the index to
 the working tree. Alternatively, using linkgit:git-restore[1]
 and specifying a commit with `--source`, you
 can copy the contents of a path out of a commit to the index and to the
 working tree in one go.
 
-'git reset' (--patch | -p) [<tree-ish>] [--] [<paths>...]::
+'git reset' (--patch | -p) [<tree-ish>] [--] [<pathspec>...]::
 	Interactively select hunks in the difference between the index
 	and `<tree-ish>` (defaults to `HEAD`).  The chosen hunks are applied
 	in reverse to the index.

@@ -101,6 +101,13 @@ OPTIONS
 	`reset.quiet` config option. `--quiet` and `--no-quiet` will
 	override the default behavior.
 
+\--::
+	Do not interpret any more arguments as options.
+
+<pathspec>...::
+	Limits the paths affected by the operation.
++
+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 EXAMPLES
 --------

index fdd572168b..9291c0fd72 100644
@@ -30,8 +30,8 @@
 
 static const char * const git_reset_usage[] = {
 	N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]"),
-	N_("git reset [-q] [<tree-ish>] [--] <paths>..."),
-	N_("git reset --patch [<tree-ish>] [--] [<paths>...]"),
+	N_("git reset [-q] [<tree-ish>] [--] <pathspec>..."),
+	N_("git reset --patch [<tree-ish>] [--] [<pathspec>...]"),
 	NULL
 };
 

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

This will be used to support the new option '--pathspec-from-file' in git add, git-commit, git reset etc.

Note also that we specifically handle CR/LF line endings to support Windows better.

To simplify code, file is first parsed into argv_array. This allows to avoid refactoring parse_pathspec().

I considered adding nul_term_line to flags instead, but decided that it doesn't fit there.

The new code is mostly taken from cmd_update_index() and split_mail_conv().

---
 pathspec.c | 38 ++++++++++++++++++++++++++++++++++++++
 pathspec.h | 10 ++++++++++
 2 files changed, 48 insertions(+)

index 12c2b322b3..128f27fcb7 100644
@@ -3,6 +3,8 @@
 #include "dir.h"
 #include "pathspec.h"
 #include "attr.h"
+#include "argv-array.h"
+#include "quote.h"
 
 /*
  * Finds which of the given pathspecs match items in the index.

@@ -613,6 +615,42 @@ void parse_pathspec(struct pathspec *pathspec,
 	}
 }
 
+void parse_pathspec_file(struct pathspec *pathspec, unsigned magic_mask,
+			 unsigned flags, const char *prefix,
+			 const char *file, int nul_term_line)
+{
+	struct argv_array parsed_file = ARGV_ARRAY_INIT;
+	strbuf_getline_fn getline_fn = nul_term_line ? strbuf_getline_nul :
+						       strbuf_getline;
+	struct strbuf buf = STRBUF_INIT;
+	struct strbuf unquoted = STRBUF_INIT;
+	FILE *in;
+
+	if (!strcmp(file, "-"))
+		in = stdin;
+	else
+		in = xfopen(file, "r");
+
+	while (getline_fn(&buf, in) != EOF) {
+		if (!nul_term_line && buf.buf[0] == '"') {
+			strbuf_reset(&unquoted);
+			if (unquote_c_style(&unquoted, buf.buf, NULL))
+				die(_("line is badly quoted: %s"), buf.buf);
+			strbuf_swap(&buf, &unquoted);
+		}
+		argv_array_push(&parsed_file, buf.buf);
+		strbuf_reset(&buf);
+	}
+
+	strbuf_release(&unquoted);
+	strbuf_release(&buf);
+	if (in != stdin)
+		fclose(in);
+
+	parse_pathspec(pathspec, magic_mask, flags, prefix, parsed_file.argv);
+	argv_array_clear(&parsed_file);
+}
+
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src)
 {
 	int i, j;

index 1c18a2c90c..a27dc81ba2 100644
@@ -85,6 +85,16 @@ void parse_pathspec(struct pathspec *pathspec,
 		    unsigned flags,
 		    const char *prefix,
 		    const char **args);
+/*
+ * Same as parse_pathspec() but uses file as input.
+ * When 'file' is exactly "-" it uses 'stdin' instead.
+ */
+void parse_pathspec_file(struct pathspec *pathspec,
+			 unsigned magic_mask,
+			 unsigned flags,
+			 const char *prefix,
+			 const char *file,
+			 int nul_term_line);
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
 void clear_pathspec(struct pathspec *);
 

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

git add shows an example of good writing, follow it. This also better disambiguates <file>... header.

---
 Documentation/git-commit.txt | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

index afa7b75a23..a0c44978ee 100644
@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<file>...]
+	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -345,12 +345,13 @@ changes to tracked files.
 \--::
 	Do not interpret any more arguments as options.
 
-<file>...::
-	When files are given on the command line, the command
-	commits the contents of the named files, without
-	recording the changes already staged.  The contents of
-	these files are also staged for the next commit on top
-	of what have been staged before.
+<pathspec>...::
+	When pathspec is given on the command line, commit the contents of
+	the files that match the pathspec without recording the changes
+	already added to the index. The contents of these files are also
+	staged for the next commit on top of what have been staged before.
++
+For more details, see the 'pathspec' entry in linkgit:gitglossary[7].
 
 :git-commit: 1
 include::date-formats.txt[]

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Decisions taken for simplicity:
1) For now, --pathspec-from-file is declared incompatible with --interactive/--patch, even when <file> is not stdin. Such use case it not really expected. Also, it would require changes to interactive_add().
2) It is not allowed to pass pathspec in both args and file.

---
 Documentation/git-commit.txt    |  16 +++-
 builtin/commit.c                |  25 +++++-
 t/t7526-commit-pathspec-file.sh | 130 ++++++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+), 5 deletions(-)
 create mode 100755 t/t7526-commit-pathspec-file.sh

index a0c44978ee..ced5a9beab 100644
@@ -13,7 +13,8 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
-	   [-i | -o] [-S[<keyid>]] [--] [<pathspec>...]
+	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
+	   [-S[<keyid>]] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------

@@ -278,6 +279,19 @@ FROM UPSTREAM REBASE" section in linkgit:git-rebase[1].)
 	already been staged. If used together with `--allow-empty`
 	paths are also not required, and an empty commit will be created.
 
+--pathspec-from-file=<file>::
+	Pathspec is passed in `<file>` instead of commandline args. If
+	`<file>` is exactly `-` then standard input is used. Pathspec
+	elements are separated by LF or CR/LF. Pathspec elements can be
+	quoted as explained for the configuration variable `core.quotePath`
+	(see linkgit:git-config[1]). See also `--pathspec-file-nul` and
+	global `--literal-pathspecs`.
+
+--pathspec-file-nul::
+	Only meaningful with `--pathspec-from-file`. Pathspec elements are
+	separated with NUL character and all other characters are taken
+	literally (including newlines and quotes).
+
 -u[<mode>]::
 --untracked-files[=<mode>]::
 	Show untracked files.

index 294dc574cd..2db2ad0de4 100644
@@ -107,9 +107,9 @@ static int all, also, interactive, patch_interactive, only, amend, signoff;
 static int edit_flag = -1; /* unspecified */
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
 static int config_commit_verbose = -1; /* unspecified */
-static int no_post_rewrite, allow_empty_message;
+static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
-static char *sign_commit;
+static char *sign_commit, *pathspec_from_file;
 
 /*
  * The default commit message cleanup mode will remove the lines

@@ -343,6 +343,23 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 		       PATHSPEC_PREFER_FULL,
 		       prefix, argv);
 
+	if (pathspec_from_file) {
+		if (interactive)
+			die(_("--pathspec-from-file is incompatible with --interactive/--patch"));
+
+		if (pathspec.nr)
+			die(_("--pathspec-from-file is incompatible with pathspec arguments"));
+
+		parse_pathspec_file(&pathspec, 0,
+				    PATHSPEC_PREFER_FULL,
+				    prefix, pathspec_from_file, pathspec_file_nul);
+	} else if (pathspec_file_nul) {
+		die(_("--pathspec-file-nul requires --pathspec-from-file"));
+	}
+
+	if (!pathspec.nr && (also || (only && !amend && !allow_empty)))
+		die(_("No paths with --include/--only does not make sense."));
+
 	if (read_cache_preload(&pathspec) < 0)
 		die(_("index file corrupt"));
 

@@ -1198,8 +1215,6 @@ static int parse_and_validate_options(int argc, const char *argv[],
 
 	if (also + only + all + interactive > 1)
 		die(_("Only one of --include/--only/--all/--interactive/--patch can be used."));
-	if (argc == 0 && (also || (only && !amend && !allow_empty)))
-		die(_("No paths with --include/--only does not make sense."));
 	cleanup_mode = get_cleanup_mode(cleanup_arg, use_editor);
 
 	handle_untracked_files_arg(s);

@@ -1513,6 +1528,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "amend", &amend, N_("amend previous commit")),
 		OPT_BOOL(0, "no-post-rewrite", &no_post_rewrite, N_("bypass post-rewrite hook")),
 		{ 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" },
+		OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+		OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
 		/* end commit contents options */
 
 		OPT_HIDDEN_BOOL(0, "allow-empty", &allow_empty,

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

Some code blocks were moved down to be able to test for pathspec.nr in the next patch. Blocks are moved as is without any changes. This is done as separate patch to reduce the amount of diffs in next patch.

---
 builtin/add.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

index dd18e5c9b6..4fabdc72e6 100644
@@ -418,10 +418,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 	if (addremove && take_worktree_changes)
 		die(_("-A and -u are mutually incompatible"));
 
-	if (!take_worktree_changes && addremove_explicit < 0 && argc)
-		/* Turn "git add pathspec..." to "git add -A pathspec..." */
-		addremove = 1;
-
 	if (!show_only && ignore_missing)
 		die(_("Option --ignore-missing can only be used together with --dry-run"));
 

@@ -434,19 +430,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 
 	hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
 
-	flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
-		 (show_only ? ADD_CACHE_PRETEND : 0) |
-		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
-		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
-		 (!(addremove || take_worktree_changes)
-		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
-
-	if (require_pathspec && argc == 0) {
-		fprintf(stderr, _("Nothing specified, nothing added.\n"));
-		fprintf(stderr, _("Maybe you wanted to say 'git add .'?\n"));
-		return 0;
-	}
-
 	/*
 	 * Check the "pathspec '%s' did not match any files" block
 	 * below before enabling new magic.

@@ -456,6 +439,23 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		       PATHSPEC_SYMLINK_LEADING_PATH,
 		       prefix, argv);
 
+	if (require_pathspec && argc == 0) {
+		fprintf(stderr, _("Nothing specified, nothing added.\n"));
+		fprintf(stderr, _("Maybe you wanted to say 'git add .'?\n"));
+		return 0;
+	}
+
+	if (!take_worktree_changes && addremove_explicit < 0 && argc)
+		/* Turn "git add pathspec..." to "git add -A pathspec..." */
+		addremove = 1;
+
+	flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
+		 (show_only ? ADD_CACHE_PRETEND : 0) |
+		 (intent_to_add ? ADD_CACHE_INTENT : 0) |
+		 (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |
+		 (!(addremove || take_worktree_changes)
+		  ? ADD_CACHE_IGNORE_REMOVAL : 0));
+
 	if (read_cache_preload(&pathspec) < 0)
 		die(_("index file corrupt"));
 

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

---
 Documentation/git-checkout.txt | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

index 2011fdbb1d..d47046e050 100644
@@ -95,12 +95,10 @@ using `--ours` or `--theirs`.  With `-m`, changes made to the working tree
 file can be discarded to re-create the original conflicted merge result.
 
 'git checkout' (-p|--patch) [<tree-ish>] [--] [<pathspec>...]::
-	This is similar to the "check out paths to the working tree
-	from either the index or from a tree-ish" mode described
-	above, but lets you use the interactive interface to show
-	the "diff" output and choose which hunks to use in the
-	result.  See below for the description of `--patch` option.
-
+	This is similar to the previous mode, but lets you use the
+	interactive interface to show the "diff" output and choose which
+	hunks to use in the result.  See below for the description of
+	`--patch` option.
 
 OPTIONS
 -------

gitgitgadget

From: Alexandr Miloslavskiy <[email protected]>

It was added in [1]. I understand that the duplicate change was not intentional and comes from an oversight.

Also, in explanation, there was only one section for two synopsis entries.

Fix both problems by removing duplicate synopsis.

<paths> vs <pathspec> is resolved in next patch.

[1] Commit b59698ae ("checkout doc: clarify command line args for "checkout paths" mode" 2017-10-11)

---
 Documentation/git-checkout.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index cf3cac0a2b..2011fdbb1d 100644
@@ -13,7 +13,6 @@ SYNOPSIS
 'git checkout' [-q] [-f] [-m] [--detach] <commit>
 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] <new_branch>] [<start_point>]
 'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...
-'git checkout' [<tree-ish>] [--] <pathspec>...
 'git checkout' (-p|--patch) [<tree-ish>] [--] [<paths>...]
 
 DESCRIPTION

@@ -79,7 +78,7 @@ be used to detach `HEAD` at the tip of the branch (`git checkout
 +
 Omitting `<branch>` detaches `HEAD` at the tip of the current branch.
 
-'git checkout' [<tree-ish>] [--] <pathspec>...::
+'git checkout' [-f|--ours|--theirs|-m|--conflict=<style>] [<tree-ish>] [--] <paths>...::
 
 	Overwrite paths in the working tree by replacing with the
 	contents in the index or in the `<tree-ish>` (most often a

gitgitgadget