{"thread":{"id":"34857","subject":"[PATCH V3] check-ignore: Add option to ignore index contents","startedAt":"2013-09-05T16:08:01Z","lastAt":"2013-09-06T16:30:04Z","messageCount":4,"participants":["Dave Williams","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"226855","messageId":"20130905160801.GA22710@opensourcesolutions.co.uk","threadId":"34857","inReplyTo":null,"subject":"[PATCH V3] check-ignore: Add option to ignore index contents","fromName":"Dave Williams","fromEmail":"dave@opensourcesolutions.co.uk","sentAt":"2013-09-05T16:08:01Z","receivedAt":"2013-09-05T16:08:01Z","isPatch":true,"sender":{"key":"dave@opensourcesolutions.co.uk","avatar":"https://avatars.githubusercontent.com/u/2284929?v=4"},"body":"check-ignore currently shows how .gitignore rules would treat untracked\npaths. Tracked paths do not generate useful output.  This prevents\ndebugging of why a path became tracked unexpectedly unless that path is\nfirst removed from the index with `git rm --cached <path>`.\n\nThis option (-i, --no-index) simply by-passes the check for the path\nbeing in the index and hence allows tracked paths to be checked too.\n\nWhilst this behaviour deviates from the characteristics of `git add` and\n`git status` its use case is unlikely to cause any user confusion.\n\nTest scripts are augmented to check this option against the standard\nignores to ensure correct behaviour.\n\nSigned-off-by: Dave Williams <dave@opensourcesolutions.co.uk>\n---\nIn V3 I have taken on board comments from Junio Hamano and Eric\nSunshine from V2 ($gmane/233660).\n\nIn particlar Junio queried my approach in builtin/git-check-ignores.c\nthat bypassed the functions that check a path is in the index as well as\navoiding reading the index in the first place.\n\nIn my view removing the bypass makes the code slightly less clear,\nrelies on the_index being initialized and the functions using it to exit\nquickly when it has no content. Nevertheless I have bowed to his better\ndomain knowledge and after undertaking brief analysis to check the\nassumptions I have removed the extra conditional steps.  This has\nsimplified the patch. The revised code appears to have the same\nbehaviour as before and passes the test script (t/t00009-ignors.sh). \n\nRegarding the test script I have tidied up the variables containing the\nseparate option switches so they dont contain leading spaces, instead I\nhave added spaces as needed when formatting the command line.  This\nnot only improves my patch but also the existing code which was a little\ninconsistent in this respect.\n\nFinally I have rebased from the latest commmit on master to pick up\nunrelated recent changes made to builtin/check-ignores.c and updated my\ncode to be consistent with this.\n\nHopefully I have put these patch notes in the right place now! Let me\nknow if not.\n\nDave\n\n\n Documentation/git-check-ignore.txt |  7 ++++\n builtin/check-ignore.c             |  6 ++-\n t/t0008-ignores.sh                 | 75 +++++++++++++++++++++++++++++++++-----\n 3 files changed, 77 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nindex d2df487..96c591f 100644\n--- a/Documentation/git-check-ignore.txt\n+++ b/Documentation/git-check-ignore.txt\n@@ -45,6 +45,13 @@ OPTIONS\n \tnot be possible to distinguish between paths which match a\n \tpattern and those which don't.\n \n+-i, --no-index::\n+\tDon't look in the index when undertaking the checks. This can\n+\tbe used to debug why a path became tracked by e.g. `git add .`\n+\tand was not ignored by the rules as expected by the user or when\n+\tdeveloping patterns including negation to match a path previously\n+\tadded with `git add -f`.\n+\n OUTPUT\n ------\n \ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex 25aa2a5..f58f384 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -5,7 +5,7 @@\n #include \"pathspec.h\"\n #include \"parse-options.h\"\n \n-static int quiet, verbose, stdin_paths, show_non_matching;\n+static int quiet, verbose, stdin_paths, show_non_matching, no_index;\n static const char * const check_ignore_usage[] = {\n \"git check-ignore [options] pathname...\",\n \"git check-ignore [options] --stdin < <list-of-paths>\",\n@@ -24,6 +24,8 @@ static const struct option check_ignore_options[] = {\n \t\t N_(\"terminate input and output records by a NUL character\")),\n \tOPT_BOOL('n', \"non-matching\", &show_non_matching,\n \t\t N_(\"show non-matching input paths\")),\n+\tOPT_BOOL('i', \"no-index\", &no_index,\n+\t\t N_(\"ignore index when checking\")),\n \tOPT_END()\n };\n \n@@ -157,7 +159,7 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\tdie(_(\"--non-matching is only valid with --verbose\"));\n \n \t/* read_cache() is only necessary so we can watch out for submodules. */\n-\tif (read_cache() < 0)\n+\tif (!no_index && read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n \tmemset(&dir, 0, sizeof(dir));\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex c29342d..760c7bf 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -66,11 +66,11 @@ test_check_ignore () {\n \n \tinit_vars &&\n \trm -f \"$HOME/stdout\" \"$HOME/stderr\" \"$HOME/cmd\" &&\n-\techo git $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $args \\\n+\techo git $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $no_index_opt $args \\\n \t\t>\"$HOME/cmd\" &&\n \techo \"$expect_code\" >\"$HOME/expected-exit-code\" &&\n \ttest_expect_code \"$expect_code\" \\\n-\t\tgit $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $args \\\n+\t\tgit $global_args check-ignore $quiet_opt $verbose_opt $non_matching_opt $no_index_opt $args \\\n \t\t>\"$HOME/stdout\" 2>\"$HOME/stderr\" &&\n \ttest_cmp \"$HOME/expected-stdout\" \"$HOME/stdout\" &&\n \tstderr_empty_on_success \"$expect_code\"\n@@ -87,6 +87,9 @@ test_check_ignore () {\n # check-ignore --verbose output is the same as normal output except\n # for the extra first column.\n #\n+# A parameter is used to determine if the tests are run with the\n+# normal case (using the index), or with the -i or --no-index option.\n+#\n # Arguments:\n #   - (optional) prereqs for this test, e.g. 'SYMLINKS'\n #   - test name\n@@ -94,19 +97,26 @@ test_check_ignore () {\n #     from the other verbosity modes is automatically inferred\n #     from this value)\n #   - code to run (should invoke test_check_ignore)\n-test_expect_success_multi () {\n+#   - index option: --index, -i or --no-index\n+test_expect_success_multiple () {\n \tprereq=\n-\tif test $# -eq 4\n+\tif test $# -eq 5\n \tthen\n \t\tprereq=$1\n \t\tshift\n \tfi\n+\tif test \"$4\" = \"--index\"\n+\tthen\n+\t\tno_index_opt=\n+\telse\n+\t\tno_index_opt=$4\n+\tfi\n \ttestname=\"$1\" expect_all=\"$2\" code=\"$3\"\n \n \texpect_verbose=$( echo \"$expect_all\" | grep -v '^::\t' )\n \texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n \n-\ttest_expect_success $prereq \"$testname\" '\n+\ttest_expect_success $prereq \"$testname${no_index_opt:+ with $no_index_opt}\" '\n \t\texpect \"$expect\" &&\n \t\teval \"$code\"\n \t'\n@@ -116,7 +126,8 @@ test_expect_success_multi () {\n \tthen\n \t\tfor quiet_opt in '-q' '--quiet'\n \t\tdo\n-\t\t\ttest_expect_success $prereq \"$testname${quiet_opt:+ with $quiet_opt}\" \"\n+\t\t\topts=\"${no_index_opt:+$no_index_opt }$quiet_opt\"\n+\t\t\ttest_expect_success $prereq \"$testname${opts:+ with $opts}\" \"\n \t\t\texpect '' &&\n \t\t\t$code\n \t\t\"\n@@ -126,7 +137,7 @@ test_expect_success_multi () {\n \n \tfor verbose_opt in '-v' '--verbose'\n \tdo\n-\t\tfor non_matching_opt in '' ' -n' ' --non-matching'\n+\t\tfor non_matching_opt in '' '-n' '--non-matching'\n \t\tdo\n \t\t\tif test -n \"$non_matching_opt\"\n \t\t\tthen\n@@ -139,12 +150,24 @@ test_expect_success_multi () {\n \t\t\t\texpect '$my_expect' &&\n \t\t\t\t$code\n \t\t\t\"\n-\t\t\topts=\"$verbose_opt$non_matching_opt\"\n+\t\t\topts=\"${no_index_opt:+$no_index_opt }$verbose_opt${non_matching_opt:+ $non_matching_opt}\"\n \t\t\ttest_expect_success $prereq \"$testname${opts:+ with $opts}\" \"$test_code\"\n \t\tdone\n \tdone\n \tverbose_opt=\n \tnon_matching_opt=\n+\tno_index_opt=\n+}\n+\n+test_expect_success_multi () {\n+\ttest_expect_success_multiple \"$@\" \"--index\"\n+}\n+\n+test_expect_success_no_index_multi () {\n+\tfor ni in '-i' '--no-index'\n+\tdo\n+\t\ttest_expect_success_multiple \"$@\" \"$ni\"\n+\tdone\n }\n \n test_expect_success 'setup' '\n@@ -288,7 +311,7 @@ test_expect_success_multi 'needs work tree' '' '\n \n # First make sure that the presence of a file in the working tree\n # does not impact results, but that the presence of a file in the\n-# index does.\n+# index does unless the --no-index option is used.\n \n for subdir in '' 'a/'\n do\n@@ -303,22 +326,42 @@ do\n \t\t\"::\t${subdir}non-existent\" \\\n \t\t\"test_check_ignore '${subdir}non-existent' 1\"\n \n+\ttest_expect_success_no_index_multi \"non-existent file $where not ignored\" \\\n+\t\t\"::\t${subdir}non-existent\" \\\n+\t\t\"test_check_ignore '${subdir}non-existent' 1\"\n+\n \ttest_expect_success_multi \"non-existent file $where ignored\" \\\n \t\t\".gitignore:1:one\t${subdir}one\" \\\n \t\t\"test_check_ignore '${subdir}one'\"\n \n+\ttest_expect_success_no_index_multi \"non-existent file $where ignored\" \\\n+\t\t\".gitignore:1:one\t${subdir}one\" \\\n+\t\t\"test_check_ignore '${subdir}one'\"\n+\n \ttest_expect_success_multi \"existing untracked file $where not ignored\" \\\n \t\t\"::\t${subdir}not-ignored\" \\\n \t\t\"test_check_ignore '${subdir}not-ignored' 1\"\n \n+\ttest_expect_success_no_index_multi \"existing untracked file $where not ignored\" \\\n+\t\t\"::\t${subdir}not-ignored\" \\\n+\t\t\"test_check_ignore '${subdir}not-ignored' 1\"\n+\n \ttest_expect_success_multi \"existing tracked file $where not ignored\" \\\n \t\t\"::\t${subdir}ignored-but-in-index\" \\\n \t\t\"test_check_ignore '${subdir}ignored-but-in-index' 1\"\n \n+\ttest_expect_success_no_index_multi \"existing tracked file $where shown as ignored\" \\\n+\t\t\".gitignore:2:ignored-*\t${subdir}ignored-but-in-index\" \\\n+\t\t\"test_check_ignore '${subdir}ignored-but-in-index'\"\n+\n \ttest_expect_success_multi \"existing untracked file $where ignored\" \\\n \t\t\".gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \\\n \t\t\"test_check_ignore '${subdir}ignored-and-untracked'\"\n \n+\ttest_expect_success_no_index_multi \"existing untracked file $where ignored\" \\\n+\t\t\".gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \\\n+\t\t\"test_check_ignore '${subdir}ignored-and-untracked'\"\n+\n \ttest_expect_success_multi \"mix of file types $where\" \\\n \"::\t${subdir}non-existent\n .gitignore:1:one\t${subdir}one\n@@ -332,6 +375,20 @@ do\n \t\t\t${subdir}ignored-but-in-index\n \t\t\t${subdir}ignored-and-untracked'\n \t\t\"\n+\n+\ttest_expect_success_no_index_multi \"mix of file types $where\" \\\n+\"::\t${subdir}non-existent\n+.gitignore:1:one\t${subdir}one\n+::\t${subdir}not-ignored\n+.gitignore:2:ignored-*\t${subdir}ignored-but-in-index\n+.gitignore:2:ignored-*\t${subdir}ignored-and-untracked\" \\\n+\t\t\"test_check_ignore '\n+\t\t\t${subdir}non-existent\n+\t\t\t${subdir}one\n+\t\t\t${subdir}not-ignored\n+\t\t\t${subdir}ignored-but-in-index\n+\t\t\t${subdir}ignored-and-untracked'\n+\t\t\"\n done\n \n # Having established the above, from now on we mostly test against\n-- \n1.8.4.rc3\n"},{"id":"226888","messageId":"xmqqbo46hpk7.fsf@gitster.dls.corp.google.com","threadId":"34857","inReplyTo":"20130905160801.GA22710@opensourcesolutions.co.uk","subject":"Re: [PATCH V3] check-ignore: Add option to ignore index contents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-05T22:27:36Z","receivedAt":"2013-09-05T22:27:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Williams <dave@opensourcesolutions.co.uk> writes:\n\n> check-ignore currently shows how .gitignore rules would treat untracked\n> paths. Tracked paths do not generate useful output.  This prevents\n> debugging of why a path became tracked unexpectedly unless that path is\n> first removed from the index with `git rm --cached <path>`.\n>\n> This option (-i, --no-index) simply by-passes the check for the path\n> being in the index and hence allows tracked paths to be checked too.\n\nNow the long option name is \"--no-index\", it makes me wonder if \"-i\"\nis a good synonym for it, and the longer I stare at it, the more\ncertain I become convinced that it is a bad choice.\n\nDo we even need a short-and-sweet one-letter option for this?  I'd\nprefer starting with only the long option.\n\nI came up with a squashable tweak to remove \"-i\" on top of this\npatch; will tentatively queue this patch with it to 'pu'.\n\n> In particlar Junio queried my approach in\n> builtin/git-check-ignores.c that bypassed the functions that check\n> a path is in the index as well as avoiding reading the index in\n> the first place.\n\nThanks.\n\nI think this version is cleaner without those \"if (!no_index)\" used\nfor special casing in the codeflow.  An empty index is a valid\nstate, in which the in-core index starts when any git program\nbegins.  Not reading the index should be the only thing necessary to\nmimick the state in which nothing has been added to the index, and\nif that is not the case, we have found a bug in the existing code.\n\n> Regarding the test script I have tidied up the variables containing the\n> separate option switches so they dont contain leading spaces, instead I\n> have added spaces as needed when formatting the command line.  This\n> not only improves my patch but also the existing code which was a little\n> inconsistent in this respect.\n\nYeah, that's very much appreciated.\n\n> Finally I have rebased from the latest commmit on master to pick up\n> unrelated recent changes made to builtin/check-ignores.c and updated my\n> code to be consistent with this.\n>\n> Hopefully I have put these patch notes in the right place now! Let me\n> know if not.\n>\n> Dave\n\nNicely done.\n"},{"id":"226917","messageId":"20130906055955.GA12622@opensourcesolutions.co.uk","threadId":"34857","inReplyTo":"xmqqbo46hpk7.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH V3] check-ignore: Add option to ignore index contents","fromName":"Dave Williams","fromEmail":"dave@opensourcesolutions.co.uk","sentAt":"2013-09-06T05:59:55Z","receivedAt":"2013-09-06T05:59:55Z","isPatch":true,"sender":{"key":"dave@opensourcesolutions.co.uk","avatar":"https://avatars.githubusercontent.com/u/2284929?v=4"},"body":"On 15:27, Thu 05 Sep 13, Junio C Hamano wrote:\n> Now the long option name is \"--no-index\", it makes me wonder if \"-i\"\n> is a good synonym for it, and the longer I stare at it, the more\n> certain I become convinced that it is a bad choice.\n> \nI had originally called it \"--ignore-index\" at which point \"-i\" made\nmore sense!\n\nDave\n"},{"id":"226948","messageId":"xmqqmwnpzzeb.fsf@gitster.dls.corp.google.com","threadId":"34857","inReplyTo":"20130906055955.GA12622@opensourcesolutions.co.uk","subject":"Re: [PATCH V3] check-ignore: Add option to ignore index contents","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-09-06T16:30:04Z","receivedAt":"2013-09-06T16:30:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Williams <dave@opensourcesolutions.co.uk> writes:\n\n> On 15:27, Thu 05 Sep 13, Junio C Hamano wrote:\n>> Now the long option name is \"--no-index\", it makes me wonder if \"-i\"\n>> is a good synonym for it, and the longer I stare at it, the more\n>> certain I become convinced that it is a bad choice.\n>> \n> I had originally called it \"--ignore-index\" at which point \"-i\" made\n> more sense!\n\nI know, but with --no-index it no longer does.\n"}]}