path: root/builtin
diff options
authorAlexandr Miloslavskiy <>2020-02-17 17:25:16 (GMT)
committerJunio C Hamano <>2020-02-19 18:56:49 (GMT)
commit5f393dc3aa62ba3e13a9890af4e769ad84812fbc (patch)
treef1f72c594844d9bdcff2bb6cacdce0899b61c611 /builtin
parent6a7aca6f016e397838926250764318b767650bd5 (diff)
rm: support the --pathspec-from-file option
Decisions taken for simplicity: 1) It is not allowed to pass pathspec in both args and file. Adjustments were needed for `if (!argc)` block: 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) Signed-off-by: Alexandr Miloslavskiy <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'builtin')
1 files changed, 22 insertions, 6 deletions
diff --git a/builtin/rm.c b/builtin/rm.c
index 19ce95a..4858631 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -235,7 +235,8 @@ static int check_local_mod(struct object_id *head, int index_only)
static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
-static int ignore_unmatch = 0;
+static int ignore_unmatch = 0, pathspec_file_nul;
+static char *pathspec_from_file;
static struct option builtin_rm_options[] = {
OPT__DRY_RUN(&show_only, N_("dry run")),
@@ -245,6 +246,8 @@ static struct option builtin_rm_options[] = {
OPT_BOOL('r', NULL, &recursive, N_("allow recursive removal")),
OPT_BOOL( 0 , "ignore-unmatch", &ignore_unmatch,
N_("exit with a zero status even if nothing matched")),
+ OPT_PATHSPEC_FROM_FILE(&pathspec_from_file),
+ OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul),
@@ -259,8 +262,24 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, builtin_rm_options,
builtin_rm_usage, 0);
- if (!argc)
- usage_with_options(builtin_rm_usage, builtin_rm_options);
+ parse_pathspec(&pathspec, 0,
+ prefix, argv);
+ if (pathspec_from_file) {
+ if (
+ die(_("--pathspec-from-file is incompatible with pathspec arguments"));
+ parse_pathspec_file(&pathspec, 0,
+ prefix, pathspec_from_file, pathspec_file_nul);
+ } else if (pathspec_file_nul) {
+ die(_("--pathspec-file-nul requires --pathspec-from-file"));
+ }
+ if (!
+ die(_("No pathspec was given. Which files should I remove?"));
if (!index_only)
@@ -270,9 +289,6 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (read_cache() < 0)
die(_("index file corrupt"));
- parse_pathspec(&pathspec, 0,
- prefix, argv);
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, &pathspec, NULL, NULL);
seen = xcalloc(, 1);