From cca5aee39229d0d54d88c7dae18dafa956473559 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Wed, 6 Nov 2019 15:51:13 +0000 Subject: [PATCH 01/13] parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul` 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. Signed-off-by: Alexandr Miloslavskiy --- parse-options.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/parse-options.h b/parse-options.h index 38a33a087ec2aa..c6cc01e715b076 100644 --- a/parse-options.h +++ b/parse-options.h @@ -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 From fea64dfbf9085b45192f91aaaabd6143ae9469bc Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Wed, 6 Nov 2019 15:51:14 +0000 Subject: [PATCH 02/13] pathspec: add new function to parse file 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()`. Co-authored-by: Johannes Schindelin Signed-off-by: Alexandr Miloslavskiy --- pathspec.c | 38 ++++++++++++++++++++++++++++++++++++++ pathspec.h | 10 ++++++++++ 2 files changed, 48 insertions(+) diff --git a/pathspec.c b/pathspec.c index 12c2b322b30a59..128f27fcb7ae8c 100644 --- a/pathspec.c +++ b/pathspec.c @@ -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; diff --git a/pathspec.h b/pathspec.h index 1c18a2c90c4148..a27dc81ba214f3 100644 --- a/pathspec.h +++ b/pathspec.h @@ -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 *); From 1182ba39535bc7c0cc4a241bd19d4c66cf6277d7 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Wed, 6 Nov 2019 15:51:15 +0000 Subject: [PATCH 03/13] doc: reset: synchronize description `git add` shows an example of good writing, follow it. Signed-off-by: Alexandr Miloslavskiy --- Documentation/git-reset.txt | 29 ++++++++++++++++++----------- builtin/reset.c | 4 ++-- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt index 97e0544d9e1e17..d517a43e738bb9 100644 --- a/Documentation/git-reset.txt +++ b/Documentation/git-reset.txt @@ -8,8 +8,8 @@ git-reset - Reset current HEAD to the specified state SYNOPSIS -------- [verse] -'git reset' [-q] [] [--] ... -'git reset' (--patch | -p) [] [--] [...] +'git reset' [-q] [] [--] ... +'git reset' (--patch | -p) [] [--] [...] 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [] DESCRIPTION @@ -19,23 +19,23 @@ In the third form, set the current branch head (`HEAD`) to ``, optionally modifying index and working tree to match. The ``/`` defaults to `HEAD` in all forms. -'git reset' [-q] [] [--] ...:: - This form resets the index entries for all `` to their - state at ``. (It does not affect the working tree or - the current branch.) +'git reset' [-q] [] [--] ...:: + This form resets the index entries for all paths that match the + `` to their state at ``. (It does not affect + the working tree or the current branch.) + -This means that `git reset ` is the opposite of `git add -`. This command is equivalent to -`git restore [--source=] --staged ...`. +This means that `git reset ` is the opposite of `git add +`. This command is equivalent to +`git restore [--source=] --staged ...`. + -After running `git reset ` to update the index entry, you can +After running `git reset ` 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) [] [--] [...]:: +'git reset' (--patch | -p) [] [--] [...]:: Interactively select hunks in the difference between the index and `` (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. + +...:: + 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 index fdd572168b51cc..9291c0fd726c54 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -30,8 +30,8 @@ static const char * const git_reset_usage[] = { N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []"), - N_("git reset [-q] [] [--] ..."), - N_("git reset --patch [] [--] [...]"), + N_("git reset [-q] [] [--] ..."), + N_("git reset --patch [] [--] [...]"), NULL }; From cea470fc91bb3af9169cea9e1ec7a705ffb93e76 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Wed, 6 Nov 2019 15:51:16 +0000 Subject: [PATCH 04/13] reset: support the `--pathspec-from-file` option Decisions taken for simplicity: 1) For now, `--pathspec-from-file` is declared incompatible with `--patch`, even when 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 Signed-off-by: Alexandr Miloslavskiy --- 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 diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt index d517a43e738bb9..932080c55d2c23 100644 --- a/Documentation/git-reset.txt +++ b/Documentation/git-reset.txt @@ -9,18 +9,20 @@ SYNOPSIS -------- [verse] 'git reset' [-q] [] [--] ... +'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] [] 'git reset' (--patch | -p) [] [--] [...] 'git reset' [--soft | --mixed [-N] | --hard | --merge | --keep] [-q] [] DESCRIPTION ----------- -In the first and second form, copy entries from `` to the index. -In the third form, set the current branch head (`HEAD`) to ``, +In the first three forms, copy entries from `` to the index. +In the last form, set the current branch head (`HEAD`) to ``, optionally modifying index and working tree to match. The ``/`` defaults to `HEAD` in all forms. 'git reset' [-q] [] [--] ...:: - This form resets the index entries for all paths that match the +'git reset' [-q] [--pathspec-from-file= [--pathspec-file-nul]] []:: + These forms reset the index entries for all paths that match the `` to their state at ``. (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=:: + Pathspec is passed in `` instead of commandline args. If + `` 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. diff --git a/builtin/reset.c b/builtin/reset.c index 9291c0fd726c54..246bf9d737de95 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -31,6 +31,7 @@ static const char * const git_reset_usage[] = { N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []"), N_("git reset [-q] [] [--] ..."), + N_("git reset [-q] [--pathspec-from-file [--pathspec-file-nul]] []"), N_("git reset --patch [] [--] [...]"), 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 */ diff --git a/t/t7107-reset-pathspec-file.sh b/t/t7107-reset-pathspec-file.sh new file mode 100755 index 00000000000000..6b1a731fffe65f --- /dev/null +++ b/t/t7107-reset-pathspec-file.sh @@ -0,0 +1,155 @@ +#!/bin/sh + +test_description='reset --pathspec-from-file' + +. ./test-lib.sh + +test_tick + +test_expect_success setup ' + echo A >fileA.t && + echo B >fileB.t && + echo C >fileC.t && + echo D >fileD.t && + git add . && + git commit --include . -m "Commit" && + git tag checkpoint +' + +restore_checkpoint () { + git reset --hard checkpoint +} + +verify_expect () { + git status --porcelain -- fileA.t fileB.t fileC.t fileD.t >actual && + test_cmp expect actual +} + +test_expect_success '--pathspec-from-file from stdin' ' + restore_checkpoint && + + git rm fileA.t && + echo fileA.t | git reset --pathspec-from-file=- && + + cat >expect <<-\EOF && + D fileA.t + EOF + verify_expect +' + +test_expect_success '--pathspec-from-file from file' ' + restore_checkpoint && + + git rm fileA.t && + echo fileA.t >list && + git reset --pathspec-from-file=list && + + cat >expect <<-\EOF && + D fileA.t + EOF + verify_expect +' + +test_expect_success 'NUL delimiters' ' + restore_checkpoint && + + git rm fileA.t fileB.t && + printf "fileA.t\0fileB.t\0" | git reset --pathspec-from-file=- --pathspec-file-nul && + + 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=- && + + 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\r\n" | git reset --pathspec-from-file=- && + + cat >expect <<-\EOF && + D fileA.t + D fileB.t + EOF + verify_expect +' + +test_expect_success 'quotes' ' + restore_checkpoint && + + git rm fileA.t && + printf "\"file\\101.t\"" | git reset --pathspec-from-file=- && + + cat >expect <<-\EOF && + D fileA.t + EOF + verify_expect +' + +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-nul && + + cat >expect <<-\EOF && + D fileA.t + EOF + test_must_fail verify_expect +' + +test_expect_success '--pathspec-from-file is not compatible with --soft or --hard' ' + restore_checkpoint && + + git rm fileA.t && + echo fileA.t >list && + test_must_fail git reset --soft --pathspec-from-file=list && + test_must_fail git reset --hard --pathspec-from-file=list +' + +test_expect_success 'only touches what was listed' ' + restore_checkpoint && + + git rm fileA.t fileB.t fileC.t fileD.t && + 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 From 0e1ac7e8a70ae0d508b71bf91a0e0b8f666cbe34 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Wed, 6 Nov 2019 15:51:17 +0000 Subject: [PATCH 05/13] doc: commit: synchronize description `git add` shows an example of good writing, follow it. This also better disambiguates ... header. Signed-off-by: Alexandr Miloslavskiy --- Documentation/git-commit.txt | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index afa7b75a23dac0..a0c44978ee0bbb 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -13,7 +13,7 @@ SYNOPSIS [-F | -m ] [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify] [-e] [--author=] [--date=] [--cleanup=] [--[no-]status] - [-i | -o] [-S[]] [--] [...] + [-i | -o] [-S[]] [--] [...] DESCRIPTION ----------- @@ -345,12 +345,13 @@ changes to tracked files. \--:: Do not interpret any more arguments as options. -...:: - 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, see the 'pathspec' entry in linkgit:gitglossary[7]. :git-commit: 1 include::date-formats.txt[] From c877866c1323f4562564fa28cd355e413ca4d6b5 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Wed, 6 Nov 2019 15:51:18 +0000 Subject: [PATCH 06/13] commit: support the --pathspec-from-file option Decisions taken for simplicity: 1) For now, `--pathspec-from-file` is declared incompatible with `--interactive/--patch`, even when 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 --- 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 diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index a0c44978ee0bbb..ced5a9beab159d 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -13,7 +13,8 @@ SYNOPSIS [-F | -m ] [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify] [-e] [--author=] [--date=] [--cleanup=] [--[no-]status] - [-i | -o] [-S[]] [--] [...] + [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]] + [-S[]] [--] [...] 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=:: + Pathspec is passed in `` instead of commandline args. If + `` 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[]:: --untracked-files[=]:: Show untracked files. diff --git a/builtin/commit.c b/builtin/commit.c index 294dc574cdda59..2db2ad0de4c4ec 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -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, diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh new file mode 100755 index 00000000000000..a06b683534898e --- /dev/null +++ b/t/t7526-commit-pathspec-file.sh @@ -0,0 +1,130 @@ +#!/bin/sh + +test_description='commit --pathspec-from-file' + +. ./test-lib.sh + +test_tick + +test_expect_success setup ' + test_commit file0 && + git tag checkpoint && + + echo A >fileA.t && + echo B >fileB.t && + echo C >fileC.t && + echo D >fileD.t && + git add fileA.t fileB.t fileC.t fileD.t +' + +restore_checkpoint () { + git reset --soft checkpoint +} + +verify_expect () { + git diff-tree --no-commit-id --name-status -r HEAD >actual && + test_cmp expect actual +} + +test_expect_success '--pathspec-from-file from stdin' ' + restore_checkpoint && + + echo fileA.t | git commit --pathspec-from-file=- -m "Commit" && + + cat >expect <<-\EOF && + A fileA.t + EOF + verify_expect +' + +test_expect_success '--pathspec-from-file from file' ' + restore_checkpoint && + + echo fileA.t >list && + git commit --pathspec-from-file=list -m "Commit" && + + cat >expect <<-\EOF && + A fileA.t + EOF + verify_expect +' + +test_expect_success 'NUL delimiters' ' + restore_checkpoint && + + printf "fileA.t\0fileB.t\0" | git commit --pathspec-from-file=- --pathspec-file-nul -m "Commit" && + + 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" && + + 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\r\n" | git commit --pathspec-from-file=- -m "Commit" && + + cat >expect <<-\EOF && + A fileA.t + A fileB.t + EOF + verify_expect +' + +test_expect_success 'quotes' ' + restore_checkpoint && + + printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" && + + cat >expect <<-\EOF && + A fileA.t + EOF + verify_expect expect +' + +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-nul -m "Commit" +' + +test_expect_success 'only touches what was listed' ' + restore_checkpoint && + + printf "fileB.t\nfileC.t\n" | git commit --pathspec-from-file=- -m "Commit" && + + cat >expect <<-\EOF && + A fileB.t + A fileC.t + EOF + verify_expect +' + +test_done From a97910cb55be86738d499786511db0ea9af9f785 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 2 Dec 2019 20:03:41 +0100 Subject: [PATCH 07/13] cmd_add: prepare for next patch 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. Signed-off-by: Alexandr Miloslavskiy --- builtin/add.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/builtin/add.c b/builtin/add.c index dd18e5c9b67038..4fabdc72e6d8dd 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -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")); From 9a62da3470b34b490717699ce40a7c73984d1cea Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 2 Dec 2019 20:04:11 +0100 Subject: [PATCH 08/13] add: support the --pathspec-from-file option Decisions taken for simplicity: 1) For now, `--pathspec-from-file` is declared incompatible with `--interactive/--patch/--edit`, even when is not `stdin`. Such use case it not really expected. Also, it would require changes to `interactive_add()` and `edit_patch()`. 2) It is not allowed to pass pathspec in both args and file. Signed-off-by: Alexandr Miloslavskiy --- Documentation/git-add.txt | 16 ++++- builtin/add.c | 30 +++++++-- t/t3704-add-pathspec-file.sh | 127 +++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 5 deletions(-) create mode 100755 t/t3704-add-pathspec-file.sh diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt index 8b0e4c7fa8c592..be5e3ac54b8587 100644 --- a/Documentation/git-add.txt +++ b/Documentation/git-add.txt @@ -11,7 +11,8 @@ SYNOPSIS 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p] [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing] [--renormalize] - [--chmod=(+|-)x] [--] [...] + [--chmod=(+|-)x] [--pathspec-from-file= [--pathspec-file-nul]] + [--] [...] DESCRIPTION ----------- @@ -187,6 +188,19 @@ for "git add --no-all ...", i.e. ignored removed files. bit is only changed in the index, the files on disk are left unchanged. +--pathspec-from-file=:: + Pathspec is passed in `` instead of commandline args. If + `` 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). + \--:: This option can be used to separate command-line options from the list of files, (useful when filenames might be mistaken diff --git a/builtin/add.c b/builtin/add.c index 4fabdc72e6d8dd..9f6b263abaaf61 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -28,6 +28,8 @@ static const char * const builtin_add_usage[] = { static int patch_interactive, add_interactive, edit_interactive; static int take_worktree_changes; static int add_renormalize; +static int pathspec_file_nul; +static const char *pathspec_from_file; struct update_callback_data { int flags; @@ -309,6 +311,8 @@ static struct option builtin_add_options[] = { N_("override the executable bit of the listed files")), OPT_HIDDEN_BOOL(0, "warn-embedded-repo", &warn_on_embedded_repo, N_("warn when adding an embedded repository")), + OPT_PATHSPEC_FROM_FILE(&pathspec_from_file), + OPT_PATHSPEC_FILE_NUL(&pathspec_file_nul), OPT_END(), }; @@ -402,11 +406,17 @@ int cmd_add(int argc, const char **argv, const char *prefix) builtin_add_usage, PARSE_OPT_KEEP_ARGV0); if (patch_interactive) add_interactive = 1; - if (add_interactive) + if (add_interactive) { + if (pathspec_from_file) + die(_("--pathspec-from-file is incompatible with --interactive/--patch")); exit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive)); + } - if (edit_interactive) + if (edit_interactive) { + if (pathspec_from_file) + die(_("--pathspec-from-file is incompatible with --edit")); return(edit_patch(argc, argv, prefix)); + } argc--; argv++; @@ -439,13 +449,25 @@ int cmd_add(int argc, const char **argv, const char *prefix) PATHSPEC_SYMLINK_LEADING_PATH, prefix, argv); - if (require_pathspec && argc == 0) { + if (pathspec_from_file) { + if (pathspec.nr) + die(_("--pathspec-from-file is incompatible with pathspec arguments")); + + parse_pathspec_file(&pathspec, PATHSPEC_ATTR, + PATHSPEC_PREFER_FULL | + PATHSPEC_SYMLINK_LEADING_PATH, + prefix, pathspec_from_file, pathspec_file_nul); + } else if (pathspec_file_nul) { + die(_("--pathspec-file-nul requires --pathspec-from-file")); + } + + if (require_pathspec && pathspec.nr == 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) + if (!take_worktree_changes && addremove_explicit < 0 && pathspec.nr) /* Turn "git add pathspec..." to "git add -A pathspec..." */ addremove = 1; diff --git a/t/t3704-add-pathspec-file.sh b/t/t3704-add-pathspec-file.sh new file mode 100755 index 00000000000000..3cfdb669b7a5a1 --- /dev/null +++ b/t/t3704-add-pathspec-file.sh @@ -0,0 +1,127 @@ +#!/bin/sh + +test_description='add --pathspec-from-file' + +. ./test-lib.sh + +test_tick + +test_expect_success setup ' + test_commit file0 && + echo A >fileA.t && + echo B >fileB.t && + echo C >fileC.t && + echo D >fileD.t +' + +restore_checkpoint () { + git reset +} + +verify_expect () { + git status --porcelain --untracked-files=no -- fileA.t fileB.t fileC.t fileD.t >actual && + test_cmp expect actual +} + +test_expect_success '--pathspec-from-file from stdin' ' + restore_checkpoint && + + echo fileA.t | git add --pathspec-from-file=- && + + cat >expect <<-\EOF && + A fileA.t + EOF + verify_expect +' + +test_expect_success '--pathspec-from-file from file' ' + restore_checkpoint && + + echo fileA.t >list && + git add --pathspec-from-file=list && + + cat >expect <<-\EOF && + A fileA.t + EOF + verify_expect +' + +test_expect_success 'NUL delimiters' ' + restore_checkpoint && + + printf "fileA.t\0fileB.t\0" | git add --pathspec-from-file=- --pathspec-file-nul && + + 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 add --pathspec-from-file=- && + + 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 add --pathspec-from-file=- && + + 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\r\n" | git add --pathspec-from-file=- && + + cat >expect <<-\EOF && + A fileA.t + A fileB.t + EOF + verify_expect +' + +test_expect_success 'quotes' ' + restore_checkpoint && + + printf "\"file\\101.t\"" | git add --pathspec-from-file=- && + + cat >expect <<-\EOF && + A fileA.t + EOF + verify_expect +' + +test_expect_success 'quotes not compatible with --pathspec-file-nul' ' + restore_checkpoint && + + printf "\"file\\101.t\"" >list && + test_must_fail git add --pathspec-from-file=list --pathspec-file-nul +' + +test_expect_success 'only touches what was listed' ' + restore_checkpoint && + + printf "fileB.t\nfileC.t\n" | git add --pathspec-from-file=- && + + cat >expect <<-\EOF && + A fileB.t + A fileC.t + EOF + verify_expect +' + +test_done From 5e449c8d29cbade125bacac6c04778ea85e3b0bd Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Mon, 2 Dec 2019 15:13:21 +0100 Subject: [PATCH 09/13] doc: checkout: remove duplicate synopsis 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. vs is resolved in next patch. [1] Commit b59698ae ("checkout doc: clarify command line args for "checkout paths" mode" 2017-10-11) Signed-off-by: Alexandr Miloslavskiy --- Documentation/git-checkout.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index cf3cac0a2b518e..2011fdbb1d1036 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -13,7 +13,6 @@ SYNOPSIS 'git checkout' [-q] [-f] [-m] [--detach] 'git checkout' [-q] [-f] [-m] [[-b|-B|--orphan] ] [] 'git checkout' [-f|--ours|--theirs|-m|--conflict=