git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>,
	Junio C Hamano <gitster@pobox.com>,
	Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
Subject: [PATCH 5/5] commit: support the --pathspec-from-file option
Date: Mon, 04 Nov 2019 19:26:45 +0000	[thread overview]
Message-ID: <f4847046896848d3f16bc5f3cb7a26271cefd97c.1572895605.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.445.git.1572895605.gitgitgadget@gmail.com>

From: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>

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.

Signed-off-by: Alexandr Miloslavskiy <alexandr.miloslavskiy@syntevo.com>
---
 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

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 4341d0e3ab..ec4752298d 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -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>]::
diff --git a/builtin/commit.c b/builtin/commit.c
index e588bc6ad3..532f305926 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_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,
diff --git a/t/t7526-commit-pathspec-file.sh b/t/t7526-commit-pathspec-file.sh
new file mode 100755
index 0000000000..c5d68e01e6
--- /dev/null
+++ b/t/t7526-commit-pathspec-file.sh
@@ -0,0 +1,107 @@
+#!/bin/sh
+
+test_description='commit --pathspec-from-file'
+
+. ./test-lib.sh
+
+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) &&
+	
+	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_state () {
+	git diff-tree --no-commit-id --name-status -r HEAD >actual &&
+	test_cmp "$1" actual
+}
+
+test_expect_success '--pathspec-from-file from stdin' '
+	restore_checkpoint &&
+
+	echo fileA.t | git commit --pathspec-from-file=- -m "Commit" &&
+
+	verify_state expect.a
+'
+
+test_expect_success '--pathspec-from-file from file' '
+	restore_checkpoint &&
+
+	echo fileA.t >list &&
+	git commit --pathspec-from-file=list -m "Commit" &&
+
+	verify_state expect.a
+'
+
+test_expect_success 'NUL delimiters' '
+	restore_checkpoint &&
+
+	printf fileA.tQfileB.t | q_to_nul | git commit --pathspec-from-file=- --pathspec-file-null -m "Commit" &&
+
+	verify_state expect.ab
+'
+
+test_expect_success 'LF delimiters' '
+	restore_checkpoint &&
+
+	printf "fileA.t\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
+
+	verify_state expect.ab
+'
+
+test_expect_success 'CRLF delimiters' '
+	restore_checkpoint &&
+
+	printf "fileA.t\r\nfileB.t" | git commit --pathspec-from-file=- -m "Commit" &&
+
+	verify_state expect.ab
+'
+
+test_expect_success 'quotes' '
+	restore_checkpoint &&
+
+	printf "\"file\\101.t\"" | git commit --pathspec-from-file=- -m "Commit" &&
+
+	verify_state expect.a
+'
+
+test_expect_success 'quotes not compatible with --pathspec-file-null' '
+	restore_checkpoint &&
+
+	printf "\"file\\101.t\"" >list &&
+	test_must_fail git commit --pathspec-from-file=list --pathspec-file-null -m "Commit"
+'
+
+test_expect_success 'only touches what was listed' '
+	restore_checkpoint &&
+
+	printf "fileB.t\nfileC.t" | git commit --pathspec-from-file=- -m "Commit" &&
+
+	verify_state expect.bc
+'
+
+test_done
-- 
gitgitgadget

  parent reply	other threads:[~2019-11-04 19:26 UTC|newest]

Thread overview: 80+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-04 19:26 [PATCH 0/5] Add --pathspec-from-file option for reset, commit Alexandr Miloslavskiy via GitGitGadget
2019-11-04 19:26 ` [PATCH 1/5] pathspec: add new function to parse file Alexandr Miloslavskiy via GitGitGadget
2019-11-05 15:02   ` Phillip Wood
2019-11-05 19:14     ` Alexandr Miloslavskiy
2019-11-06 15:56     ` Alexandr Miloslavskiy
2019-11-04 19:26 ` [PATCH 2/5] doc: reset: unify <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-11-06  4:01   ` Junio C Hamano
2019-11-06 15:56     ` Alexandr Miloslavskiy
2019-11-07  5:46       ` Junio C Hamano
2019-11-07 11:05         ` Alexandr Miloslavskiy
2019-11-08  3:04           ` Junio C Hamano
2019-11-04 19:26 ` [PATCH 3/5] reset: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-11-05 15:03   ` Phillip Wood
2019-11-05 19:22     ` Phillip Wood
2019-11-05 19:36     ` Alexandr Miloslavskiy
2019-11-06 15:56     ` Alexandr Miloslavskiy
2019-11-05 16:14   ` Phillip Wood
2019-11-05 19:37     ` Alexandr Miloslavskiy
2019-11-06  4:40   ` Junio C Hamano
2019-11-06 15:56     ` Alexandr Miloslavskiy
2019-11-04 19:26 ` [PATCH 4/5] doc: commit: unify <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-11-06  4:50   ` Junio C Hamano
2019-11-06 15:56     ` Alexandr Miloslavskiy
2019-11-07  5:54       ` Junio C Hamano
2019-11-07 11:39         ` Alexandr Miloslavskiy
2019-11-04 19:26 ` Alexandr Miloslavskiy via GitGitGadget [this message]
2019-11-05 16:27   ` [PATCH 5/5] commit: support the --pathspec-from-file option Phillip Wood
2019-11-05 19:42     ` Alexandr Miloslavskiy
2019-11-06 15:56     ` Alexandr Miloslavskiy
2019-12-10 10:42       ` Phillip Wood
2019-12-11 11:43         ` Alexandr Miloslavskiy
2019-12-11 14:27           ` Phillip Wood
2019-12-11 15:06             ` Alexandr Miloslavskiy
2019-12-11 16:14               ` Junio C Hamano
2019-12-11 16:20                 ` Alexandr Miloslavskiy
2019-12-12 14:56             ` Alexandr Miloslavskiy
2019-11-06  4:51   ` Junio C Hamano
2019-11-06 15:51 ` [PATCH v2 0/6] Add --pathspec-from-file option for reset, commit Alexandr Miloslavskiy via GitGitGadget
2019-11-06 15:51   ` [PATCH v2 1/6] parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul` Alexandr Miloslavskiy via GitGitGadget
2019-11-06 15:51   ` [PATCH v2 2/6] pathspec: add new function to parse file Alexandr Miloslavskiy via GitGitGadget
2019-11-19  5:59     ` Junio C Hamano
2019-11-19 16:50       ` Alexandr Miloslavskiy
2019-11-06 15:51   ` [PATCH v2 3/6] doc: reset: unify <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-11-19  6:05     ` Junio C Hamano
2019-11-19 16:52       ` Alexandr Miloslavskiy
2019-11-06 15:51   ` [PATCH v2 4/6] reset: support the `--pathspec-from-file` option Alexandr Miloslavskiy via GitGitGadget
2019-11-06 15:51   ` [PATCH v2 5/6] doc: commit: unify <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-11-19  6:16     ` Junio C Hamano
2019-11-19 16:53       ` Alexandr Miloslavskiy
2019-11-19 17:02       ` Alexandr Miloslavskiy
2019-11-06 15:51   ` [PATCH v2 6/6] commit: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-11-19  6:10     ` Junio C Hamano
2019-11-19 16:56       ` Alexandr Miloslavskiy
2019-11-19 16:48   ` [PATCH v3 0/6] Add --pathspec-from-file option for reset, commit Alexandr Miloslavskiy via GitGitGadget
2019-11-19 16:48     ` [PATCH v3 1/6] parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul` Alexandr Miloslavskiy via GitGitGadget
2019-11-19 16:48     ` [PATCH v3 2/6] pathspec: add new function to parse file Alexandr Miloslavskiy via GitGitGadget
2019-11-19 16:48     ` [PATCH v3 3/6] doc: reset: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-11-19 16:48     ` [PATCH v3 4/6] reset: support the `--pathspec-from-file` option Alexandr Miloslavskiy via GitGitGadget
2019-11-19 16:48     ` [PATCH v3 5/6] doc: commit: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-11-19 16:48     ` [PATCH v3 6/6] commit: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-11-20  4:04     ` [PATCH v3 0/6] Add --pathspec-from-file option for reset, commit Junio C Hamano
2019-11-20  9:22       ` Alexandr Miloslavskiy
2019-12-03 14:02     ` [PATCH v4 00/13] Add --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 01/13] parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul` Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 02/13] pathspec: add new function to parse file Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 03/13] doc: reset: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 04/13] reset: support the `--pathspec-from-file` option Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 05/13] doc: commit: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 06/13] commit: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 07/13] cmd_add: prepare for next patch Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 08/13] add: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 09/13] doc: checkout: remove duplicate synopsis Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 10/13] doc: checkout: fix broken text reference Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 11/13] doc: checkout: synchronize <pathspec> description Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 12/13] doc: restore: " Alexandr Miloslavskiy via GitGitGadget
2019-12-03 14:02       ` [PATCH v4 13/13] checkout, restore: support the --pathspec-from-file option Alexandr Miloslavskiy via GitGitGadget
2019-12-03 16:55       ` [PATCH v4 00/13] Add " Junio C Hamano
2019-12-03 17:06         ` Alexandr Miloslavskiy
2019-12-04 19:25           ` Junio C Hamano
2019-12-05 10:43             ` Alexandr Miloslavskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f4847046896848d3f16bc5f3cb7a26271cefd97c.1572895605.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=alexandr.miloslavskiy@syntevo.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).