git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 1/3] add: add new --exclude option to git add
@ 2015-03-15 13:49 Alexander Kuleshov
  2015-03-15 13:50 ` [PATCH 2/3] Documentation/git-add.txt: describe --exclude option Alexander Kuleshov
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Alexander Kuleshov @ 2015-03-15 13:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, Alexander Kuleshov

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"),
+	  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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/3] Documentation/git-add.txt: describe --exclude option
  2015-03-15 13:49 [PATCH RFC 1/3] add: add new --exclude option to git add Alexander Kuleshov
@ 2015-03-15 13:50 ` Alexander Kuleshov
  2015-03-15 18:14   ` Eric Sunshine
  2015-03-15 13:50 ` [PATCH 3/3] t3700-add: added test for " Alexander Kuleshov
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Alexander Kuleshov @ 2015-03-15 13:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, Alexander Kuleshov

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 Documentation/git-add.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/3] t3700-add: added test for --exclude option
  2015-03-15 13:49 [PATCH RFC 1/3] add: add new --exclude option to git add Alexander Kuleshov
  2015-03-15 13:50 ` [PATCH 2/3] Documentation/git-add.txt: describe --exclude option Alexander Kuleshov
@ 2015-03-15 13:50 ` Alexander Kuleshov
  2015-03-15 18:00   ` Torsten Bögershausen
  2015-03-15 18:18   ` Eric Sunshine
  2015-03-15 17:39 ` [PATCH RFC 1/3] add: add new --exclude option to git add Philip Oakley
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 16+ messages in thread
From: Alexander Kuleshov @ 2015-03-15 13:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git@vger.kernel.org, Alexander Kuleshov

Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
---
 t/t3700-add.sh | 7 +++++++
 1 file changed, 7 insertions(+)

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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 13:49 [PATCH RFC 1/3] add: add new --exclude option to git add Alexander Kuleshov
  2015-03-15 13:50 ` [PATCH 2/3] Documentation/git-add.txt: describe --exclude option Alexander Kuleshov
  2015-03-15 13:50 ` [PATCH 3/3] t3700-add: added test for " Alexander Kuleshov
@ 2015-03-15 17:39 ` Philip Oakley
  2015-03-15 17:51 ` Torsten Bögershausen
  2015-03-15 18:12 ` Eric Sunshine
  4 siblings, 0 replies; 16+ messages in thread
From: Philip Oakley @ 2015-03-15 17:39 UTC (permalink / raw)
  To: Alexander Kuleshov, Junio C Hamano; +Cc: git, Alexander Kuleshov

From: "Alexander Kuleshov" <kuleshovmail@gmail.com>
> 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/   ??

> +   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 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 13:49 [PATCH RFC 1/3] add: add new --exclude option to git add Alexander Kuleshov
                   ` (2 preceding siblings ...)
  2015-03-15 17:39 ` [PATCH RFC 1/3] add: add new --exclude option to git add Philip Oakley
@ 2015-03-15 17:51 ` Torsten Bögershausen
  2015-03-15 18:00   ` Alexander Kuleshov
  2015-03-15 18:03   ` Torsten Bögershausen
  2015-03-15 18:12 ` Eric Sunshine
  4 siblings, 2 replies; 16+ messages in thread
From: Torsten Bögershausen @ 2015-03-15 17:51 UTC (permalink / raw)
  To: Alexander Kuleshov, Junio C Hamano; +Cc: git@vger.kernel.org

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?
> 
> 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 ?
> +}
> +
>  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
> +	  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 ?
>  	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  ?
>  			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;
>  }
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 17:51 ` Torsten Bögershausen
@ 2015-03-15 18:00   ` Alexander Kuleshov
  2015-03-16  6:38     ` Torsten Bögershausen
  2015-03-15 18:03   ` Torsten Bögershausen
  1 sibling, 1 reply; 16+ messages in thread
From: Alexander Kuleshov @ 2015-03-15 18:00 UTC (permalink / raw)
  To: Torsten Bögershausen, philipoakley
  Cc: Junio C Hamano, git@vger.kernel.org

Hello All,

>> s /no/not/   ??

Thank you Philip.

2015-03-15 23:51 GMT+06:00 Torsten Bögershausen <tboegi@web.de>:
> 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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] t3700-add: added test for --exclude option
  2015-03-15 13:50 ` [PATCH 3/3] t3700-add: added test for " Alexander Kuleshov
@ 2015-03-15 18:00   ` Torsten Bögershausen
  2015-03-15 18:18   ` Eric Sunshine
  1 sibling, 0 replies; 16+ messages in thread
From: Torsten Bögershausen @ 2015-03-15 18:00 UTC (permalink / raw)
  To: Alexander Kuleshov, Junio C Hamano; +Cc: git@vger.kernel.org

> +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)

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 17:51 ` Torsten Bögershausen
  2015-03-15 18:00   ` Alexander Kuleshov
@ 2015-03-15 18:03   ` Torsten Bögershausen
  1 sibling, 0 replies; 16+ messages in thread
From: Torsten Bögershausen @ 2015-03-15 18:03 UTC (permalink / raw)
  To: Torsten Bögershausen, Alexander Kuleshov, Junio C Hamano
  Cc: git@vger.kernel.org

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 13:49 [PATCH RFC 1/3] add: add new --exclude option to git add Alexander Kuleshov
                   ` (3 preceding siblings ...)
  2015-03-15 17:51 ` Torsten Bögershausen
@ 2015-03-15 18:12 ` Eric Sunshine
  2015-03-15 20:09   ` Junio C Hamano
  4 siblings, 1 reply; 16+ messages in thread
From: Eric Sunshine @ 2015-03-15 18:12 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: Junio C Hamano, git@vger.kernel.org

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.

> 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()?

> +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?

>         { 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/3] Documentation/git-add.txt: describe --exclude option
  2015-03-15 13:50 ` [PATCH 2/3] Documentation/git-add.txt: describe --exclude option Alexander Kuleshov
@ 2015-03-15 18:14   ` Eric Sunshine
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Sunshine @ 2015-03-15 18:14 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: Junio C Hamano, git@vger.kernel.org

On Sun, Mar 15, 2015 at 9:50 AM, Alexander Kuleshov
<kuleshovmail@gmail.com> wrote:
> 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.

> +
>  \--::
>         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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] t3700-add: added test for --exclude option
  2015-03-15 13:50 ` [PATCH 3/3] t3700-add: added test for " Alexander Kuleshov
  2015-03-15 18:00   ` Torsten Bögershausen
@ 2015-03-15 18:18   ` Eric Sunshine
  1 sibling, 0 replies; 16+ messages in thread
From: Eric Sunshine @ 2015-03-15 18:18 UTC (permalink / raw)
  To: Alexander Kuleshov; +Cc: Junio C Hamano, git@vger.kernel.org

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/

> 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?

> +'
> +
>  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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 18:12 ` Eric Sunshine
@ 2015-03-15 20:09   ` Junio C Hamano
  2015-03-15 21:12     ` Philip Oakley
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2015-03-15 20:09 UTC (permalink / raw)
  To: Eric Sunshine; +Cc: Alexander Kuleshov, git@vger.kernel.org

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...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 20:09   ` Junio C Hamano
@ 2015-03-15 21:12     ` Philip Oakley
  2015-03-15 21:50       ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Philip Oakley @ 2015-03-15 21:12 UTC (permalink / raw)
  To: Junio C Hamano, Eric Sunshine; +Cc: Alexander Kuleshov, git

From: "Junio C Hamano" <gitster@pobox.com>
> 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?

>
> 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 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 21:12     ` Philip Oakley
@ 2015-03-15 21:50       ` Junio C Hamano
  2015-03-16  7:11         ` Alexander Kuleshov
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2015-03-15 21:50 UTC (permalink / raw)
  To: Philip Oakley; +Cc: Eric Sunshine, Alexander Kuleshov, git

"Philip Oakley" <philipoakley@iee.org> writes:

> 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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 18:00   ` Alexander Kuleshov
@ 2015-03-16  6:38     ` Torsten Bögershausen
  0 siblings, 0 replies; 16+ messages in thread
From: Torsten Bögershausen @ 2015-03-16  6:38 UTC (permalink / raw)
  To: Alexander Kuleshov, philipoakley; +Cc: Junio C Hamano, git@vger.kernel.org


> 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...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH RFC 1/3] add: add new --exclude option to git add
  2015-03-15 21:50       ` Junio C Hamano
@ 2015-03-16  7:11         ` Alexander Kuleshov
  0 siblings, 0 replies; 16+ messages in thread
From: Alexander Kuleshov @ 2015-03-16  7:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Philip Oakley, Eric Sunshine, git@vger.kernel.org

>> Isn't the problem one of "how are users to discover such magic".

Yes it was main reason.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-03-16  7:11 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-15 13:49 [PATCH RFC 1/3] add: add new --exclude option to git add Alexander Kuleshov
2015-03-15 13:50 ` [PATCH 2/3] Documentation/git-add.txt: describe --exclude option Alexander Kuleshov
2015-03-15 18:14   ` Eric Sunshine
2015-03-15 13:50 ` [PATCH 3/3] t3700-add: added test for " Alexander Kuleshov
2015-03-15 18:00   ` Torsten Bögershausen
2015-03-15 18:18   ` Eric Sunshine
2015-03-15 17:39 ` [PATCH RFC 1/3] add: add new --exclude option to git add Philip Oakley
2015-03-15 17:51 ` Torsten Bögershausen
2015-03-15 18:00   ` Alexander Kuleshov
2015-03-16  6:38     ` Torsten Bögershausen
2015-03-15 18:03   ` Torsten Bögershausen
2015-03-15 18:12 ` Eric Sunshine
2015-03-15 20:09   ` Junio C Hamano
2015-03-15 21:12     ` Philip Oakley
2015-03-15 21:50       ` Junio C Hamano
2015-03-16  7:11         ` Alexander Kuleshov

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 NNTP newsgroup(s).