threads / rfc / 38812

RFC patch, 3 partsadd: add new --exclude option to git add

Subject: [PATCH RFC 1/3] add: add new --exclude option to git add

## tl;dr

16 messages between Mar 15, 2015 and Mar 16, 2015. Diffs are folded; open one to read it.

replies: 15people: 5as markdown or json

Alexander Kuleshov· Mar 15, 2015, 13:49 UTC · lore

This patch introduces new --exclude option for the git add command.

We already have core.excludesfile configuration variable which indicates a path to file which contains patterns to exclude. This patch provides ability to pass --exclude option to the git add command to exclude paths from command line in addition to which found in the ignore files.

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 builtin/add.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
Show changes to builtin/add.c +20 −0
diff --git a/builtin/add.c b/builtin/add.c
index 3390933..5c602a6 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -244,6 +244,16 @@ static int ignore_removal_cb(const struct option *opt, const char *arg, int unse
 	return 0;
 }
 
+struct string_list exclude_list = STRING_LIST_INIT_NODUP;
+struct exclude_list *el;
+
+static int exclude_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct string_list *exclude_list = opt->value;
+	string_list_append(exclude_list, arg);
+	return 0;
+}
+
 static struct option builtin_add_options[] = {
 	OPT__DRY_RUN(&show_only, N_("dry run")),
 	OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -255,6 +265,9 @@ static struct option builtin_add_options[] = {
 	OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
 	OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
 	OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
+	{ OPTION_CALLBACK, 0, "exclude", &exclude_list, N_("pattern"),
+	  N_("do no add files matching pattern to index"),
+	  0, exclude_cb },
 	{ OPTION_CALLBACK, 0, "ignore-removal", &addremove_explicit,
 	  NULL /* takes no arguments */,
 	  N_("ignore paths removed in the working tree (same as --no-all)"),
@@ -298,6 +311,7 @@ static int add_files(struct dir_struct *dir, int flags)
 
 int cmd_add(int argc, const char **argv, const char *prefix)
 {
+	int i;
 	int exit_status = 0;
 	struct pathspec pathspec;
 	struct dir_struct dir;
@@ -381,6 +395,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		if (!ignored_too) {
 			dir.flags |= DIR_COLLECT_IGNORED;
 			setup_standard_excludes(&dir);
+
+			el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
+			for (i = 0; i < exclude_list.nr; i++)
+				add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
+
 		}
 
 		memset(&empty_pathspec, 0, sizeof(empty_pathspec));
@@ -446,5 +465,6 @@ finish:
 			die(_("Unable to write new index file"));
 	}
 
+	string_list_clear(&exclude_list, 0);
 	return exit_status;
 }

-- 
2.3.3.472.g20ceeac
Alexander Kuleshov· Mar 15, 2015, 13:50 UTC · re: Alexander Kuleshov · lore

[PATCH 2/3] Documentation/git-add.txt: describe --exclude option

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 Documentation/git-add.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Show changes to Documentation/git-add.txt +5 −1
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index f2eb907..4bc156a 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
-	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
+	  [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--exclude=<pattern>]
 	  [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing]
 	  [--] [<pathspec>...]
 
@@ -164,6 +164,10 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
 	be ignored, no matter if they are already present in the work
 	tree or not.
 
+--exclude=<pattern>::
+	Do not add files to the index in addition which are found in
+	the .gitignore.
+
 \--::
 	This option can be used to separate command-line options from
 	the list of files, (useful when filenames might be mistaken

-- 
2.3.3.472.g20ceeac
Eric Sunshine· Mar 15, 2015, 18:14 UTC · re: Alexander Kuleshov · lore

Re: [PATCH 2/3] Documentation/git-add.txt: describe --exclude option

On Sun, Mar 15, 2015 at 9:50 AM, Alexander Kuleshov <kuleshovmail@gmail.com> wrote:

Show 22 quoted lines
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index f2eb907..4bc156a 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -9,7 +9,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git add' [--verbose | -v] [--dry-run | -n] [--force | -f] [--interactive | -i] [--patch | -p]
> -         [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
> +         [--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]] [--exclude=<pattern>]
>           [--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing]
>           [--] [<pathspec>...]
>
> @@ -164,6 +164,10 @@ for "git add --no-all <pathspec>...", i.e. ignored removed files.
>         be ignored, no matter if they are already present in the work
>         tree or not.
>
> +--exclude=<pattern>::
> +       Do not add files to the index in addition which are found in
> +       the .gitignore.
This is difficult to understand. Perhaps something like:
    Also ignore files matching <pattern>, a .gitignore-like
    pattern.

This option can be specified multiple times, can't it? The documentation should say so.

Show 6 quoted lines
> +
>  \--::
>         This option can be used to separate command-line options from
>         the list of files, (useful when filenames might be mistaken
> --
> 2.3.3.472.g20ceeac
Alexander Kuleshov· Mar 15, 2015, 13:50 UTC · re: Alexander Kuleshov · lore

[PATCH 3/3] t3700-add: added test for --exclude option

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 t/t3700-add.sh | 7 +++++++
 1 file changed, 7 insertions(+)
Show changes to t/t3700-add.sh +7 −0
diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index f7ff1f5..c52a5d0 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -81,6 +81,13 @@ test_expect_success '.gitignore is honored' '
 	! (git ls-files | grep "\\.ig")
 '
 
+test_expect_success 'Test that "git add --exclude" works' '
+	touch foo &&
+	touch bar &&
+	git add --exclude="bar" . &&
+	! (git ls-files | grep bar)
+'
+
 test_expect_success 'error out when attempting to add ignored ones without -f' '
 	test_must_fail git add a.?? &&
 	! (git ls-files | grep "\\.ig")

-- 
2.3.3.472.g20ceeac
Torsten Bögershausen· Mar 15, 2015, 18:00 UTC · re: Alexander Kuleshov · lore

Re: [PATCH 3/3] t3700-add: added test for --exclude option

> +test_expect_success 'Test that "git add --exclude" works' '
> +	touch foo &&
> +	touch bar &&
can be written shorter as
        >foo &&
	>bar &&
> +	git add --exclude="bar" . &&
Side question:
Do we need "" here ?
Or should we test files with white space as well, like this:
        >foo &&
	>bar &&
	>"b a z" &&
    	git add --exclude="bar" --exclude="b a z" . &&
	echo bar >expect &&
	git ls-files >actual &&
	test_cmp expect actual
(Which doesn't use ! grep, but test_cmp, which will give more information when
the test case does not pass)
Eric Sunshine· Mar 15, 2015, 18:18 UTC · re: Alexander Kuleshov · lore

Re: [PATCH 3/3] t3700-add: added test for --exclude option

On Sun, Mar 15, 2015 at 9:50 AM, Alexander Kuleshov <kuleshovmail@gmail.com> wrote:

> t3700-add: added test for --exclude option
Write in imperative mood: s/added/add/
Show 13 quoted lines
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> diff --git a/t/t3700-add.sh b/t/t3700-add.sh
> index f7ff1f5..c52a5d0 100755
> --- a/t/t3700-add.sh
> +++ b/t/t3700-add.sh
> @@ -81,6 +81,13 @@ test_expect_success '.gitignore is honored' '
>         ! (git ls-files | grep "\\.ig")
>  '
>
> +test_expect_success 'Test that "git add --exclude" works' '
> +       touch foo &&
> +       touch bar &&

Expanding slightly on what Torsten said: Use 'touch' when the timestamp of the file is significant to the test; otherwise, just use ">foo".

> +       git add --exclude="bar" . &&
> +       ! (git ls-files | grep bar)

For completeness, don't you also want to test that "foo" _does_ appear in the git-ls-files output?

Show 7 quoted lines
> +'
> +
>  test_expect_success 'error out when attempting to add ignored ones without -f' '
>         test_must_fail git add a.?? &&
>         ! (git ls-files | grep "\\.ig")
> --
> 2.3.3.472.g20ceeac
Philip Oakley· Mar 15, 2015, 17:34 UTC · re: Alexander Kuleshov · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

From: "Alexander Kuleshov" <kuleshovmail@gmail.com>
Show 47 quoted lines
> This patch introduces new --exclude option for the git add
> command.
>
> We already have core.excludesfile configuration variable which 
> indicates
> a path to file which contains patterns to exclude. This patch provides
> ability to pass --exclude option to the git add command to exclude 
> paths
> from command line in addition to which found in the ignore files.
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> builtin/add.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 3390933..5c602a6 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -244,6 +244,16 @@ static int ignore_removal_cb(const struct option 
> *opt, const char *arg, int unse
>  return 0;
> }
>
> +struct string_list exclude_list = STRING_LIST_INIT_NODUP;
> +struct exclude_list *el;
> +
> +static int exclude_cb(const struct option *opt, const char *arg, int 
> unset)
> +{
> + struct string_list *exclude_list = opt->value;
> + string_list_append(exclude_list, arg);
> + return 0;
> +}
> +
> static struct option builtin_add_options[] = {
>  OPT__DRY_RUN(&show_only, N_("dry run")),
>  OPT__VERBOSE(&verbose, N_("be verbose")),
> @@ -255,6 +265,9 @@ static struct option builtin_add_options[] = {
>  OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked 
> files")),
>  OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the 
> fact that the path will be added later")),
>  OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all 
> tracked and untracked files")),
> + { OPTION_CALLBACK, 0, "exclude", &exclude_list, N_("pattern"),
> +   N_("do no add files matching pattern to index"),
s /no/not/   ??
Show 36 quoted lines
> +   0, exclude_cb },
>  { OPTION_CALLBACK, 0, "ignore-removal", &addremove_explicit,
>    NULL /* takes no arguments */,
>    N_("ignore paths removed in the working tree (same as --no-all)"),
> @@ -298,6 +311,7 @@ static int add_files(struct dir_struct *dir, int 
> flags)
>
> int cmd_add(int argc, const char **argv, const char *prefix)
> {
> + int i;
>  int exit_status = 0;
>  struct pathspec pathspec;
>  struct dir_struct dir;
> @@ -381,6 +395,11 @@ int cmd_add(int argc, const char **argv, const 
> char *prefix)
>  if (!ignored_too) {
>  dir.flags |= DIR_COLLECT_IGNORED;
>  setup_standard_excludes(&dir);
> +
> + el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
> + for (i = 0; i < exclude_list.nr; i++)
> + add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
> +
>  }
>
>  memset(&empty_pathspec, 0, sizeof(empty_pathspec));
> @@ -446,5 +465,6 @@ finish:
>  die(_("Unable to write new index file"));
>  }
>
> + string_list_clear(&exclude_list, 0);
>  return exit_status;
> }
> -- 
> 2.3.3.472.g20ceeac
>

-- Philip

Torsten Bögershausen· Mar 15, 2015, 17:51 UTC · re: Alexander Kuleshov · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

On 2015-03-15 14.49, Alexander Kuleshov wrote:
Thanks for working on Git, some minor remarks/suggestions inline.
> This patch introduces new --exclude option for the git add
> command.

"This patch" is redundant. Shorter may be: Introduce the --exclude option for git add

> 
> We already have core.excludesfile configuration variable which indicates
> a path to file which contains patterns to exclude. This patch provides
same here: Provide the ability to pass ....
> ability to pass --exclude option to the git add command to exclude paths
> from command line in addition to which found in the ignore files.
"found" ?? Would "specified" be better?
Show 22 quoted lines
> 
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  builtin/add.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/builtin/add.c b/builtin/add.c
> index 3390933..5c602a6 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -244,6 +244,16 @@ static int ignore_removal_cb(const struct option *opt, const char *arg, int unse
>  	return 0;
>  }
>  
> +struct string_list exclude_list = STRING_LIST_INIT_NODUP;
> +struct exclude_list *el;
> +
> +static int exclude_cb(const struct option *opt, const char *arg, int unset)
> +{
> +	struct string_list *exclude_list = opt->value;
> +	string_list_append(exclude_list, arg);
> +	return 0;
When we always return 0, the function can be void ?
Show 10 quoted lines
> +}
> +
>  static struct option builtin_add_options[] = {
>  	OPT__DRY_RUN(&show_only, N_("dry run")),
>  	OPT__VERBOSE(&verbose, N_("be verbose")),
> @@ -255,6 +265,9 @@ static struct option builtin_add_options[] = {
>  	OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
>  	OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
>  	OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
> +	{ OPTION_CALLBACK, 0, "exclude", &exclude_list, N_("pattern"),

What does pattern mean ? Is it the same as a "pathspec", used in Documentation/git-add.txt

Show 10 quoted lines
> +	  N_("do no add files matching pattern to index"),
> +	  0, exclude_cb },
>  	{ OPTION_CALLBACK, 0, "ignore-removal", &addremove_explicit,
>  	  NULL /* takes no arguments */,
>  	  N_("ignore paths removed in the working tree (same as --no-all)"),
> @@ -298,6 +311,7 @@ static int add_files(struct dir_struct *dir, int flags)
>  
>  int cmd_add(int argc, const char **argv, const char *prefix)
>  {
> +	int i;
Do we need "i" here ?
Show 5 quoted lines
>  	int exit_status = 0;
>  	struct pathspec pathspec;
>  	struct dir_struct dir;
> @@ -381,6 +395,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>  		if (!ignored_too) {
or could it be declared here  ?
Show 18 quoted lines
>  			dir.flags |= DIR_COLLECT_IGNORED;
>  			setup_standard_excludes(&dir);
> +
> +			el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
> +			for (i = 0; i < exclude_list.nr; i++)
> +				add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
> +
>  		}
>  
>  		memset(&empty_pathspec, 0, sizeof(empty_pathspec));
> @@ -446,5 +465,6 @@ finish:
>  			die(_("Unable to write new index file"));
>  	}
>  
> +	string_list_clear(&exclude_list, 0);
>  	return exit_status;
>  }
>
Alexander Kuleshov· Mar 15, 2015, 18:00 UTC · re: Torsten Bögershausen · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

Hello All,
>> s /no/not/   ??
Thank you Philip.
2015-03-15 23:51 GMT+06:00 Torsten Bögershausen <tboegi@web.de>:
Show 10 quoted lines
> On 2015-03-15 14.49, Alexander Kuleshov wrote:
>
> Thanks for working on Git, some minor remarks/suggestions inline.
>> This patch introduces new --exclude option for the git add
>> command.
> "This patch" is redundant. Shorter may be:
> Introduce the --exclude option for git add
>
>>....
>>
Thank you Torsten for you feedback. I will make all fixes and resend patch.

One little question, how to better resend it? Just send v2 for the 1/3 or resend all with v2? Or maybe will be better to make one patch from these 3 pathes?

Thank you.
Torsten Bögershausen· Mar 16, 2015, 06:38 UTC · re: Alexander Kuleshov · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

Show 5 quoted lines
> One little question, how to better resend it? Just send v2 for the 1/3
> or resend all with v2? Or maybe will be better to make one patch from
> these 3 pathes?
>
> Thank you.
My personal suggestion would be:

Please wait 24 hours to collect feedback from the different time-zones in the world, where we have Git developers :-)

Until the reviews has settled, you can send small updated pieces or piecelets/snipplets of the code which is discussed.

When there is a kind of agreement, send a complete serious, with V3/V4...
Torsten Bögershausen· Mar 15, 2015, 18:03 UTC · re: Torsten Bögershausen · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

On 2015-03-15 18.51, Torsten Bögershausen wrote:
>>  	OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
>> +	{ OPTION_CALLBACK, 0, "exclude", &exclude_list, N_("pattern"),
> What does pattern mean ?

I was too fast, take that back: Documentation/gitignore.txt uses pattern as well. Sorry for the noise.

Eric Sunshine· Mar 15, 2015, 18:12 UTC · re: Alexander Kuleshov · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

In addition to points raised by Philip and Torsten...

On Sun, Mar 15, 2015 at 9:49 AM, Alexander Kuleshov <kuleshovmail@gmail.com> wrote:

> add: add new --exclude option to git add
No need for redundant "to git add", since you already have the "add:" prefix.
> This patch introduces new --exclude option for the git add
> command.
This line merely repeats the Subject: line, thus can be dropped.
> We already have core.excludesfile configuration variable which indicates
> a path to file which contains patterns to exclude. This patch provides
> ability to pass --exclude option to the git add command to exclude paths
> from command line in addition to which found in the ignore files.

The commit message is missing the important justification for why this new option is desirable, and why only git-add needs it.

Show 11 quoted lines
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
> diff --git a/builtin/add.c b/builtin/add.c
> index 3390933..5c602a6 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -244,6 +244,16 @@ static int ignore_removal_cb(const struct option *opt, const char *arg, int unse
>         return 0;
>  }
>
> +struct string_list exclude_list = STRING_LIST_INIT_NODUP;
Shouldn't this be declared static?
> +struct exclude_list *el;
Why is this declared globally when it's only needed locally by cmd_add()?
Show 17 quoted lines
> +static int exclude_cb(const struct option *opt, const char *arg, int unset)
> +{
> +       struct string_list *exclude_list = opt->value;
> +       string_list_append(exclude_list, arg);
> +       return 0;
> +}
> +
>  static struct option builtin_add_options[] = {
>         OPT__DRY_RUN(&show_only, N_("dry run")),
>         OPT__VERBOSE(&verbose, N_("be verbose")),
> @@ -255,6 +265,9 @@ static struct option builtin_add_options[] = {
>         OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")),
>         OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that the path will be added later")),
>         OPT_BOOL('A', "all", &addremove_explicit, N_("add changes from all tracked and untracked files")),
> +       { OPTION_CALLBACK, 0, "exclude", &exclude_list, N_("pattern"),
> +         N_("do no add files matching pattern to index"),
> +         0, exclude_cb },
Can this just be OPT_STRING_LIST instead of OPTION_CALLBACK?
Show 32 quoted lines
>         { OPTION_CALLBACK, 0, "ignore-removal", &addremove_explicit,
>           NULL /* takes no arguments */,
>           N_("ignore paths removed in the working tree (same as --no-all)"),
> @@ -298,6 +311,7 @@ static int add_files(struct dir_struct *dir, int flags)
>
>  int cmd_add(int argc, const char **argv, const char *prefix)
>  {
> +       int i;
>         int exit_status = 0;
>         struct pathspec pathspec;
>         struct dir_struct dir;
> @@ -381,6 +395,11 @@ int cmd_add(int argc, const char **argv, const char *prefix)
>                 if (!ignored_too) {
>                         dir.flags |= DIR_COLLECT_IGNORED;
>                         setup_standard_excludes(&dir);
> +
> +                       el = add_exclude_list(&dir, EXC_CMDL, "--exclude option");
> +                       for (i = 0; i < exclude_list.nr; i++)
> +                               add_exclude(exclude_list.items[i].string, "", 0, el, -(i+1));
> +
>                 }
>
>                 memset(&empty_pathspec, 0, sizeof(empty_pathspec));
> @@ -446,5 +465,6 @@ finish:
>                         die(_("Unable to write new index file"));
>         }
>
> +       string_list_clear(&exclude_list, 0);
>         return exit_status;
>  }
> --
> 2.3.3.472.g20ceeac
Junio C Hamano· Mar 15, 2015, 20:09 UTC · re: Eric Sunshine · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

Eric Sunshine <sunshine@sunshineco.com> writes:
> The commit message is missing the important justification for why this
> new option is desirable, and why only git-add needs it.

I think that is a very good point. I actually do not see why this option is ever needed, in a modern world that has the negative pathspec magic.

Is there a reason why this is undesiable
    $ git add -- \*.c ':!secret.c'
and has to be spelled as
    $ git add --exclude=secret.c -- \*.c
I do not see why...
Philip Oakley· Mar 15, 2015, 20:45 UTC · re: Junio C Hamano · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

From: "Junio C Hamano" <gitster@pobox.com>
Show 9 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> The commit message is missing the important justification for why 
>> this
>> new option is desirable, and why only git-add needs it.
>
> I think that is a very good point.  I actually do not see why this
> option is ever needed, in a modern world that has the negative
> pathspec magic.

Isn't the problem one of "how are users to discover such magic". The fact we call it 'magic' (a sleight of hand...) may be why Alexander felt the need for the extra option.

Maybe He/We would be better off adjusting the documentation such that these 'magic' capabilities are brought out of their hiding places into regular view - e.g. a paragraph within the 'git add' documentation (and/or other commands) showing how such excludes are easily done with a few simple keystrokes....

'git help revisions' doesn't appear to cover it. I'm not sure we even mention "negative pathspec" in the documentation (apart from rel notes 1.9.0 & 2.3.0).

Maybe Alexander could change itch to: make the "magic negative pathspec" capability more visible?

Show 11 quoted lines
>
> Is there a reason why this is undesiable
>
>    $ git add -- \*.c ':!secret.c'
>
> and has to be spelled as
>
>    $ git add --exclude=secret.c -- \*.c
>
> I do not see why...
> --
Philip
Junio C Hamano· Mar 15, 2015, 21:50 UTC · re: Philip Oakley · lore

Re: [PATCH RFC 1/3] add: add new --exclude option to git add

"Philip Oakley" <philipoakley@iee.org> writes:
Show 5 quoted lines
> Maybe He/We would be better off adjusting the documentation such that
> these 'magic' capabilities are brought out of their hiding places into
> regular view - e.g. a paragraph within the 'git add' documentation
> (and/or other commands) showing how such excludes are easily done with
> a few simple keystrokes....

I agree documenting is good, but I do not think it belongs to "git add". For example, shouldn't "git log -- \*.c ':!secret.c'" work the same way?

> 'git help revisions' doesn't appear to cover it.

Of course not, because "revisions" would cover revisions, and pathspecs are different animals ;-) Again, I agree there should be a section somewhere close to the central place that talks about pathspecs in general and then talk about pathspec magic there.

There are some references in git(1) but they talk about ways to implicitly enable the magic to _all_ pathspecs, before introducing the readers to what the magics are and how they can use them.

Perhaps in git(1), before we introduce "GIT COMMANDS", we may want to have a new section that talks about the general structure of the Git command line, laying out the general rules such as

 * dashed options come first
 * revisions and ranges come second
 * pathspecs come last

The first one is described in detail in gitcli(7) and how revisions are spelled is given in gitrevisions(7). How pathspecs are spelled and used may want to be its own gitpathspecs(7), or if there are not too many rules, can be spelled out there inline. Either way, we would want a brief section in git(1) to serve as a jumping board for the former two, at least. Also descriptions for --literal-pathspecs and friends would want to refer to either the section or have a direct reference to gitpathspecs(7) if we are going to write it a new file.

← back to recent threads