* [PATCH] check-ignore: Add option to ignore index contents
@ 2013-08-29 22:46 Dave Williams
2013-08-30 0:10 ` Adam Spiers
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Dave Williams @ 2013-08-29 22:46 UTC (permalink / raw)
To: git; +Cc: Adam Spiers
check-ignore currently shows how .gitignore rules would treat untracked
paths. Tracked paths do not generate useful output. This prevents
debugging of why a path became tracked unexpectedly unless that path is
first removed from the index with git rm --cached <path>
This option (-i, --ignore-index) simply by-passes the check for the path
being in the index and hence allows tracked path to be checked too.
Whilst this behaviour deviates from the characteristics of git add and
git status its use case is unlikely to cause any user confusion.
Signed-off-by: Dave Williams <dave@opensourcesolutions.co.uk>
---
Documentation/git-check-ignore.txt | 6 ++++++
builtin/check-ignore.c | 10 +++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt
index d2df487..bb878ff 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -45,6 +45,12 @@ OPTIONS
not be possible to distinguish between paths which match a
pattern and those which don't.
+-i, --ignore-index::
+ Don't look in the index when undertaking the checks. This means
+ the results deviate from those seen by git add and git status
+ but is useful when understanding why a path became tracked by
+ e.g. git add . and was not ignored as expected by the rules.
+
OUTPUT
------
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 4a8fc70..c8f6ae1 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -5,7 +5,7 @@
#include "pathspec.h"
#include "parse-options.h"
-static int quiet, verbose, stdin_paths, show_non_matching;
+static int quiet, verbose, stdin_paths, show_non_matching, ignore_index;
static const char * const check_ignore_usage[] = {
"git check-ignore [options] pathname...",
"git check-ignore [options] --stdin < <list-of-paths>",
@@ -24,6 +24,8 @@ static const struct option check_ignore_options[] = {
N_("input paths are terminated by a null character")),
OPT_BOOLEAN('n', "non-matching", &show_non_matching,
N_("show non-matching input paths")),
+ OPT_BOOLEAN('i', "ignore-index", &ignore_index,
+ N_("ignore index when checking")),
OPT_END()
};
@@ -82,7 +84,9 @@ static int check_ignore(struct dir_struct *dir,
* should not be ignored, in order to be consistent with
* 'git status', 'git add' etc.
*/
- seen = find_pathspecs_matching_against_index(pathspec);
+ if (!ignore_index) {
+ seen = find_pathspecs_matching_against_index(pathspec);
+ }
for (i = 0; pathspec[i]; i++) {
path = pathspec[i];
full_path = prefix_path(prefix, prefix
@@ -90,7 +94,7 @@ static int check_ignore(struct dir_struct *dir,
full_path = check_path_for_gitlink(full_path);
die_if_path_beyond_symlink(full_path, prefix);
exclude = NULL;
- if (!seen[i]) {
+ if (ignore_index || !seen[i]) {
exclude = last_exclude_matching(dir, full_path, &dtype);
}
if (!quiet && (exclude || show_non_matching))
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] check-ignore: Add option to ignore index contents
2013-08-29 22:46 [PATCH] check-ignore: Add option to ignore index contents Dave Williams
@ 2013-08-30 0:10 ` Adam Spiers
2013-08-30 0:51 ` Duy Nguyen
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Adam Spiers @ 2013-08-30 0:10 UTC (permalink / raw)
To: git; +Cc: Dave Williams
On Thu, Aug 29, 2013 at 11:46:52PM +0100, Dave Williams wrote:
> check-ignore currently shows how .gitignore rules would treat untracked
> paths. Tracked paths do not generate useful output. This prevents
> debugging of why a path became tracked unexpectedly unless that path is
> first removed from the index with git rm --cached <path>
>
> This option (-i, --ignore-index) simply by-passes the check for the path
> being in the index and hence allows tracked path to be checked too.
>
> Whilst this behaviour deviates from the characteristics of git add and
> git status its use case is unlikely to cause any user confusion.
>
> Signed-off-by: Dave Williams <dave@opensourcesolutions.co.uk>
[snipped]
Sounds like a great idea to me, and the code changes look reasonable;
however you forgot to add corresponding tests :-) Please can you
expand the patch to include them?
But I'm in favour in principle - thanks a lot for working on this!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] check-ignore: Add option to ignore index contents
2013-08-29 22:46 [PATCH] check-ignore: Add option to ignore index contents Dave Williams
2013-08-30 0:10 ` Adam Spiers
@ 2013-08-30 0:51 ` Duy Nguyen
2013-08-30 4:13 ` Junio C Hamano
2013-08-30 0:57 ` Junio C Hamano
2013-08-30 6:51 ` Johannes Sixt
3 siblings, 1 reply; 6+ messages in thread
From: Duy Nguyen @ 2013-08-30 0:51 UTC (permalink / raw)
To: Dave Williams; +Cc: Git Mailing List, Adam Spiers
On Fri, Aug 30, 2013 at 5:46 AM, Dave Williams
<dave@opensourcesolutions.co.uk> wrote:
> check-ignore currently shows how .gitignore rules would treat untracked
> paths. Tracked paths do not generate useful output. This prevents
> debugging of why a path became tracked unexpectedly unless that path is
> first removed from the index with git rm --cached <path>
>
> This option (-i, --ignore-index) simply by-passes the check for the path
> being in the index and hence allows tracked path to be checked too.
Maybe --no-index, already used by git-diff and git-grep.
> Whilst this behaviour deviates from the characteristics of git add and
> git status its use case is unlikely to cause any user confusion.
--
Duy
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] check-ignore: Add option to ignore index contents
2013-08-29 22:46 [PATCH] check-ignore: Add option to ignore index contents Dave Williams
2013-08-30 0:10 ` Adam Spiers
2013-08-30 0:51 ` Duy Nguyen
@ 2013-08-30 0:57 ` Junio C Hamano
2013-08-30 6:51 ` Johannes Sixt
3 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2013-08-30 0:57 UTC (permalink / raw)
To: Dave Williams; +Cc: git, Adam Spiers
Dave Williams <dave@opensourcesolutions.co.uk> writes:
> check-ignore currently shows how .gitignore rules would treat untracked
> paths. Tracked paths do not generate useful output. This prevents
> debugging of why a path became tracked unexpectedly unless that path is
> first removed from the index with git rm --cached <path>
>
> This option (-i, --ignore-index) simply by-passes the check for the path
> being in the index and hence allows tracked path to be checked too.
>
> Whilst this behaviour deviates from the characteristics of git add and
> git status its use case is unlikely to cause any user confusion.
>
> Signed-off-by: Dave Williams <dave@opensourcesolutions.co.uk>
> ---
> Documentation/git-check-ignore.txt | 6 ++++++
> builtin/check-ignore.c | 10 +++++++---
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt
> index d2df487..bb878ff 100644
> --- a/Documentation/git-check-ignore.txt
> +++ b/Documentation/git-check-ignore.txt
> @@ -45,6 +45,12 @@ OPTIONS
> not be possible to distinguish between paths which match a
> pattern and those which don't.
>
> +-i, --ignore-index::
> + Don't look in the index when undertaking the checks. This means
> + the results deviate from those seen by git add and git status
> + but is useful when understanding why a path became tracked by
> + e.g. git add . and was not ignored as expected by the rules.
Please think twice after you write "This means", "What this means
is", etc. to see if you can get rid of sentence before that, which
needed such clarification. If the sentence can stand on its own,
and what follows "This means" is also useful information, then it is
a sign that "What this means is" is better rephrased as "Note that".
Also, I think `git add .` should be quoted to make it clear where
the sample command begins and ends. Using a pair of `` would be the
most appropriate here.
> OUTPUT
> ------
>
> diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
> index 4a8fc70..c8f6ae1 100644
> --- a/builtin/check-ignore.c
> +++ b/builtin/check-ignore.c
> @@ -5,7 +5,7 @@
> #include "pathspec.h"
> #include "parse-options.h"
>
> -static int quiet, verbose, stdin_paths, show_non_matching;
> +static int quiet, verbose, stdin_paths, show_non_matching, ignore_index;
> static const char * const check_ignore_usage[] = {
> "git check-ignore [options] pathname...",
> "git check-ignore [options] --stdin < <list-of-paths>",
> @@ -24,6 +24,8 @@ static const struct option check_ignore_options[] = {
> N_("input paths are terminated by a null character")),
> OPT_BOOLEAN('n', "non-matching", &show_non_matching,
> N_("show non-matching input paths")),
> + OPT_BOOLEAN('i', "ignore-index", &ignore_index,
> + N_("ignore index when checking")),
> OPT_END()
> };
>
> @@ -82,7 +84,9 @@ static int check_ignore(struct dir_struct *dir,
> * should not be ignored, in order to be consistent with
> * 'git status', 'git add' etc.
> */
> - seen = find_pathspecs_matching_against_index(pathspec);
> + if (!ignore_index) {
> + seen = find_pathspecs_matching_against_index(pathspec);
> + }
> for (i = 0; pathspec[i]; i++) {
> path = pathspec[i];
> full_path = prefix_path(prefix, prefix
> @@ -90,7 +94,7 @@ static int check_ignore(struct dir_struct *dir,
> full_path = check_path_for_gitlink(full_path);
> die_if_path_beyond_symlink(full_path, prefix);
> exclude = NULL;
> - if (!seen[i]) {
> + if (ignore_index || !seen[i]) {
> exclude = last_exclude_matching(dir, full_path, &dtype);
> }
> if (!quiet && (exclude || show_non_matching))
Hmph, if ignoring what is in the index is what the new feature wants
to do, why does it even need to touch check_ignore() function at all
and add a global variable ignore_index to keep track of that state?
Isn't it sufficient to skip the call to read_cache() when that
option is given?
You would need to add tests for this, too.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] check-ignore: Add option to ignore index contents
2013-08-30 0:51 ` Duy Nguyen
@ 2013-08-30 4:13 ` Junio C Hamano
0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2013-08-30 4:13 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Dave Williams, Git Mailing List, Adam Spiers
Duy Nguyen <pclouds@gmail.com> writes:
> On Fri, Aug 30, 2013 at 5:46 AM, Dave Williams
> <dave@opensourcesolutions.co.uk> wrote:
>> check-ignore currently shows how .gitignore rules would treat untracked
>> paths. Tracked paths do not generate useful output. This prevents
>> debugging of why a path became tracked unexpectedly unless that path is
>> first removed from the index with git rm --cached <path>
>>
>> This option (-i, --ignore-index) simply by-passes the check for the path
>> being in the index and hence allows tracked path to be checked too.
>
> Maybe --no-index, already used by git-diff and git-grep.
I am somewhat ashamed that I didn't think of it myself when I saw
the patch, but I agree that would be a good change for consistency.
>
>> Whilst this behaviour deviates from the characteristics of git add and
>> git status its use case is unlikely to cause any user confusion.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] check-ignore: Add option to ignore index contents
2013-08-29 22:46 [PATCH] check-ignore: Add option to ignore index contents Dave Williams
` (2 preceding siblings ...)
2013-08-30 0:57 ` Junio C Hamano
@ 2013-08-30 6:51 ` Johannes Sixt
3 siblings, 0 replies; 6+ messages in thread
From: Johannes Sixt @ 2013-08-30 6:51 UTC (permalink / raw)
To: Dave Williams; +Cc: git, Adam Spiers
Am 8/30/2013 0:46, schrieb Dave Williams:
> +-i, --ignore-index::
> + Don't look in the index when undertaking the checks. This means
> + the results deviate from those seen by git add and git status
> + but is useful when understanding why a path became tracked by
> + e.g. git add . and was not ignored as expected by the rules.
Did you mean to say "was not ignored as expected by the user"? Because the
"the rules" cannot "expect" something, they only "are applied".
-- Hannes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-30 6:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-29 22:46 [PATCH] check-ignore: Add option to ignore index contents Dave Williams
2013-08-30 0:10 ` Adam Spiers
2013-08-30 0:51 ` Duy Nguyen
2013-08-30 4:13 ` Junio C Hamano
2013-08-30 0:57 ` Junio C Hamano
2013-08-30 6:51 ` Johannes Sixt
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).