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

Subsystems: the rest

STALE3730d

12 messages, 4 authors, 2016-06-15 · open the first message on its own page

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

From: Alexander Kuleshov <hidden>
Date: 2016-06-15 23:04:09

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

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

From: Alexander Kuleshov <hidden>
Date: 2016-06-15 23:04:09

Signed-off-by: Alexander Kuleshov <redacted>
---
 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

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

From: Alexander Kuleshov <hidden>
Date: 2016-06-15 23:04:09

Signed-off-by: Alexander Kuleshov <redacted>
---
 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

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

From: Philip Oakley <hidden>
Date: 2016-06-15 23:04:09

From: "Alexander Kuleshov" <redacted>

quoted hunk

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

quoted hunk

+   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

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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:04:09

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?

quoted hunk

Signed-off-by: Alexander Kuleshov <redacted>
---
 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 ?

quoted hunk

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

quoted hunk

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

quoted hunk

 	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  ?

quoted hunk

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

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

From: Alexander Kuleshov <hidden>
Date: 2016-06-15 23:04:09

Hello All,

quoted

s /no/not/   ??

Thank you Philip.

2015-03-15 23:51 GMT+06:00 Torsten Bögershausen [off-list ref]:

On 2015-03-15 14.49, Alexander Kuleshov wrote:

Thanks for working on Git, some minor remarks/suggestions inline.

quoted

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

quoted

....

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.

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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:04:09

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

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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:04:09

On 2015-03-15 18.51, Torsten Bögershausen wrote:

quoted

 	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.

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

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:09

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

On Sun, Mar 15, 2015 at 9:49 AM, Alexander Kuleshov
[off-list ref] 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.

quoted hunk

Signed-off-by: Alexander Kuleshov <redacted>
---

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

quoted hunk

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

quoted hunk

        { 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

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

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:09

On Sun, Mar 15, 2015 at 9:50 AM, Alexander Kuleshov
[off-list ref] wrote:

quoted hunk

Signed-off-by: Alexander Kuleshov <redacted>
---

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

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

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:04:09

On Sun, Mar 15, 2015 at 9:50 AM, Alexander Kuleshov
[off-list ref] wrote:

t3700-add: added test for --exclude option

Write in imperative mood: s/added/add/

quoted hunk

Signed-off-by: Alexander Kuleshov <redacted>
---

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

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

From: Torsten Bögershausen <hidden>
Date: 2016-06-15 23:04:10

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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help