Support --pathspec-from-file in rm, stash

Changes since V1
------------------
Some polishing based on code review in V1
1) Improved error message for the case where pathspec is not given to git rm
2) Removed explicit variable initialization to 0 / NULL
3) Polishing in docs for git stash

------------------
This series continues the effort to support --pathspec-from-file in various git commands. Series already in master: [1][2]

Cc'ing Paul-Sebastian Ungureanu because I touched his git stash code.

[1] https://public-inbox.org/git/[email protected]/
[2] https://lore.kernel.org/git/[email protected]/

Alexandr Miloslavskiy (8):
doc: rm: synchronize <pathspec> description
rm: support the --pathspec-from-file option
doc: stash: split options from description (1)
doc: stash: split options from description (2)
doc: stash: document more options
doc: stash: synchronize <pathspec> description
stash: eliminate crude option parsing
stash push: support the --pathspec-from-file option

 Documentation/git-rm.txt       |  61 +++++++-------
 Documentation/git-stash.txt    | 144 +++++++++++++++++++++++----------
 builtin/rm.c                   |  28 +++++--
 builtin/stash.c                |  79 +++++++++---------
 t/t3601-rm-pathspec-file.sh    |  79 ++++++++++++++++++
 t/t3903-stash.sh               |   5 ++
 t/t3909-stash-pathspec-file.sh | 100 +++++++++++++++++++++++
 7 files changed, 381 insertions(+), 115 deletions(-)
 create mode 100755 t/t3601-rm-pathspec-file.sh
 create mode 100755 t/t3909-stash-pathspec-file.sh

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

Range-diff vs v2:

 1:  2e8c8ad8158 = 1:  cf065e905dc doc: rm: synchronize <pathspec> description
 2:  7ccbab52e51 ! 2:  7c657dea89e rm: support the --pathspec-from-file option

@@ -5,17 +5,36 @@
Decisions taken for simplicity:
1) It is not allowed to pass pathspec in both args and file.

- if (!argc) block was adapted to work with --pathspec-from-file. For
- that, I also had to parse pathspec earlier. Now it happens before
- read_cache() / hold_locked_index() / setup_work_tree(), which
- sounds fine to me.
+ Adjustments were needed for if (!argc) block:

- In case of empty pathspec, there is now a clear error message instead
- of showing usage. As a consequence, exit code has also changed. Judging
- from [1] it doesn't seem that showing usage in this case was important
- (according to commit message, it was to avoid segfault), and it doesn't
- fit into how other commands react to empty pathspec. Finally, the new
- error message is easier to understand.
+ This code actually means "pathspec is not present". Previously, pathspec
+ could only come from commandline arguments, so testing for argc was a
+ valid way of testing for the presence of pathspec. But this is no longer
+ true with --pathspec-from-file.
+
+ During the entire --pathspec-from-file story, I tried to keep its
+ behavior very close to giving pathspec on commandline, so that switching
+ from one to another doesn't involve any surprises.
+
+ However, throwing usage at user in the case of empty
+ --pathspec-from-file would puzzle because there's nothing wrong with
+ "usage" (that is, argc/argv array).
+
+ On the other hand, throwing usage in the old case also feels bad to me.
+ While it's less of a puzzle, I (as user) never liked the experience of
+ comparing my commandline to "usage", trying to spot a difference. Since
+ it's already known what the error is, it feels a lot better to give that
+ specific error to user.
+
+ Judging from [1] it doesn't seem that showing usage in this case was
+ important (the patch was to avoid segfault), and it doesn't fit into how
+ other commands react to empty pathspec (see for example git add with a
+ custom message).
+
+ Therefore, I decided to show new error text in both cases. In order to
+ continue testing for error early, I moved parse_pathspec() higher. Now
+ it happens before read_cache() / hold_locked_index() /
+ setup_work_tree(), which shouldn't cause any issues.

[1] Commit 7612a1ef ("git-rm: honor -n flag" 2006-06-09)

@@ -136,7 +155,7 @@
+ echo D >fileD.t &&
+ git add fileA.t fileB.t fileC.t fileD.t &&
+ git commit -m "files" &&
-+
++
+ git tag checkpoint
+'
+
@@ -193,7 +212,7 @@
+
+ test_must_fail git rm --pathspec-file-nul 2>err &&
+ test_i18ngrep -e "--pathspec-file-nul requires --pathspec-from-file" err &&
-+
++
+ >empty_list &&
+ test_must_fail git rm --pathspec-from-file=empty_list 2>err &&
+ test_i18ngrep -e "No pathspec was given. Which files should I remove?" err

 3:  8c212fc0ed4 = 3:  bb300215d49 doc: stash: split options from description (1)
 4:  db3a96720ce = 4:  fdaf4532404 doc: stash: split options from description (2)
 5:  f91ec08b472 = 5:  764b8668d10 doc: stash: document more options
 6:  04e2fd5865f = 6:  7353b06e30e doc: stash: synchronize <pathspec> description
 7:  0558cbbe38e = 7:  d34eaf4a272 stash: eliminate crude option parsing
 8:  0c6f28dc68d = 8:  6465a292b55 stash push: support the --pathspec-from-file option

gitgitgadget