{"thread":{"id":"63461","subject":"[PATCH RFC] diff --no-index: teach option to exclude files by pattern","startedAt":"2025-05-14T20:40:21Z","lastAt":"2025-05-15T20:24:48Z","messageCount":5,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"518070","messageId":"20250514204014.3106177-1-jacob.e.keller@intel.com","threadId":"63461","inReplyTo":null,"subject":"[PATCH RFC] diff --no-index: teach option to exclude files by pattern","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-14T20:40:14Z","receivedAt":"2025-05-14T20:40:21Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe --no-index option of git-diff enables using the diff machinery when\noperating outside a repository. This mode of git diff is able to compare\ndirectories and produce a diff of their contents. In certain cases, it\nmay be helpful to ignore certain portions of a directory.\n\nIn particular, git diff --no-index does not ignore '.git' or other VCS\n'hidden' folders. If you pass a path which happens to be a git\nrepository, it will attempt to diff the entire repository folder.\nStandard diff utilities often include options to exclude files by path.\nFor example, the GNU diffutils program has the '-x' option to exclude\nfiles by a pattern match.\n\nTeach git diff --no-index the ability to exclude files by wildmatch\npattern when recursing through directories. The '--exclude' option\nbuilds up a string list containing the patterns. These are checked with\nwildmatch() in the read_directory_contents function. If any pattern\nmatches, then the file is not included in the directory contents.\n\nThe --exclude option is only supported by the --no-index mode. Standard\ndiff modes support negative pathspecs which is more powerful. I tried to\nsee if there was a way to add support for negative pathspecs themselves,\nbut haven't yet figured out if this is possible.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\nI came across an issue when attempting to use git diff --no-index as a diff\nreplacement. I wanted to compare two folders, and one of them happened to be\na git repository. This caused the diff to recursively go into the .git\nfolder and try to compare all the paths.\n\nOther diff tools such as the diff from GNU difftools have options to exclude\nfiles by pattern match such as '-x'.\n\nGit has negative pathspecs, but this doesn't work with git diff --no-index,\nand making them work seems tricky, so I thought it wouldn't be too hard to\nimplement an exclude feature for git diff --no-index.\n\nIs something like this worthwhile? I guess I could always try to use a\nregular diff tool in cases where I'm operating on a potential repository..\nBut I like the colorization and other options of git diff and it would be\nconvenient to be able to still use git diff in this case.\n\nThoughts? What about trying to extend the tool to read in negative\npathspecs? That seems like its significantly more difficult, but might feel\nmore natural than the --exclude option.\n\nSent as RFC since this lacks tests and documentation.\n\n diff-no-index.c | 32 +++++++++++++++++++++++++-------\n 1 file changed, 25 insertions(+), 7 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 6f277892d3ae..913b2f3f7869 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -17,8 +17,10 @@\n #include \"parse-options.h\"\n #include \"string-list.h\"\n #include \"dir.h\"\n+#include \"wildmatch.h\"\n \n-static int read_directory_contents(const char *path, struct string_list *list)\n+static int read_directory_contents(const char *path, struct string_list *list,\n+\t\t\t\t   struct string_list *exclude_patterns)\n {\n \tDIR *dir;\n \tstruct dirent *e;\n@@ -26,8 +28,20 @@ static int read_directory_contents(const char *path, struct string_list *list)\n \tif (!(dir = opendir(path)))\n \t\treturn error(\"Could not open directory %s\", path);\n \n-\twhile ((e = readdir_skip_dot_and_dotdot(dir)))\n+\twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n+\t\tint skip = 0;\n+\n+\t\tfor (int i = 0; i < exclude_patterns->nr; i++) {\n+\t\t\tif (!wildmatch(exclude_patterns->items[i].string, e->d_name, 0)) {\n+\t\t\t\tskip = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tif (skip)\n+\t\t\tcontinue;\n+\n \t\tstring_list_insert(list, e->d_name);\n+\t}\n \n \tclosedir(dir);\n \treturn 0;\n@@ -130,7 +144,8 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode,\n }\n \n static int queue_diff(struct diff_options *o,\n-\t\t      const char *name1, const char *name2, int recursing)\n+\t\t      const char *name1, const char *name2, int recursing,\n+\t\t      struct string_list *exclude_patterns)\n {\n \tint mode1 = 0, mode2 = 0;\n \tenum special special1 = SPECIAL_NONE, special2 = SPECIAL_NONE;\n@@ -170,9 +185,9 @@ static int queue_diff(struct diff_options *o,\n \t\tint i1, i2, ret = 0;\n \t\tsize_t len1 = 0, len2 = 0;\n \n-\t\tif (name1 && read_directory_contents(name1, &p1))\n+\t\tif (name1 && read_directory_contents(name1, &p1, exclude_patterns))\n \t\t\treturn -1;\n-\t\tif (name2 && read_directory_contents(name2, &p2)) {\n+\t\tif (name2 && read_directory_contents(name2, &p2, exclude_patterns)) {\n \t\t\tstring_list_clear(&p1, 0);\n \t\t\treturn -1;\n \t\t}\n@@ -217,7 +232,7 @@ static int queue_diff(struct diff_options *o,\n \t\t\t\tn2 = buffer2.buf;\n \t\t\t}\n \n-\t\t\tret = queue_diff(o, n1, n2, 1);\n+\t\t\tret = queue_diff(o, n1, n2, 1, exclude_patterns);\n \t\t}\n \t\tstring_list_clear(&p1, 0);\n \t\tstring_list_clear(&p2, 0);\n@@ -306,10 +321,13 @@ int diff_no_index(struct rev_info *revs,\n \tconst char *paths[2];\n \tchar *to_free[ARRAY_SIZE(paths)] = { 0 };\n \tstruct strbuf replacement = STRBUF_INIT;\n+\tstruct string_list exclude_patterns = STRING_LIST_INIT_NODUP;\n \tconst char *prefix = revs->prefix;\n \tstruct option no_index_options[] = {\n \t\tOPT_BOOL_F(0, \"no-index\", &no_index, \"\",\n \t\t\t   PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),\n+\t\tOPT_STRING_LIST(0, \"exclude\", &exclude_patterns, N_(\"pattern\"),\n+\t\t\t\tN_(\"exclude files matching pattern when recursing a directory\")),\n \t\tOPT_END(),\n \t};\n \tstruct option *options;\n@@ -354,7 +372,7 @@ int diff_no_index(struct rev_info *revs,\n \tsetup_diff_pager(&revs->diffopt);\n \trevs->diffopt.flags.exit_with_status = 1;\n \n-\tif (queue_diff(&revs->diffopt, paths[0], paths[1], 0))\n+\tif (queue_diff(&revs->diffopt, paths[0], paths[1], 0, &exclude_patterns))\n \t\tgoto out;\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"1/\", \"2/\");\n \tdiffcore_std(&revs->diffopt);\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"518072","messageId":"xmqqzffe7vbh.fsf@gitster.g","threadId":"63461","inReplyTo":"20250514204014.3106177-1-jacob.e.keller@intel.com","subject":"Re: [PATCH RFC] diff --no-index: teach option to exclude files by pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-14T21:10:10Z","receivedAt":"2025-05-14T21:10:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n>\n> Teach git diff --no-index the ability to exclude files by wildmatch\n> pattern when recursing through directories. The '--exclude' option\n> builds up a string list containing the patterns. These are checked with\n> wildmatch() in the read_directory_contents function. If any pattern\n> matches, then the file is not included in the directory contents.\n\nA quite natural question that comes to mind is:\n\n    How would we do this for the normal \"git diff\" that is not the\n    bolted on '--no-index' mode?\n\nbut ...\n\n> The --exclude option is only supported by the --no-index mode. Standard\n> diff modes support negative pathspecs which is more powerful. I tried to\n> see if there was a way to add support for negative pathspecs themselves,\n> but haven't yet figured out if this is possible.\n\n... of course you have thought about it already.  I do agree with\nyou that we should figure out how and teach this mode to also use\npathspec, not necessarily only the negative ones but positive ones.\n\nAfter all,\n\n    $ git diff --no-index [<option>...] dirA dirB\n\nis like running\n\n    $ diff -r [<option>...] dirA dirB\n\nafter preparing these two directories like so:\n\n    $ git archive revA | ( mkdir dirA && tar Cxf dirA - )\n    $ git archive revB | ( mkdir dirB && tar Cxf dirB - )\n\nHence it is natural for users to expect that anything you can do\nwith\n\n    $ git diff revA revB\n\nshould be doable, in\n\n    $ git diff --no-index dirA dirB\n\nand vice versa.  And as you said, when comparing two revisions,\nyou'd use pathspec for this kind of thing.\n\n    $ git diff revA revB -- Documentation/ t/ ':!po/'\n\nSo, I pretty much agree with the need to be able to exclude some\nparts of the tree(s) from comparison in \"diff --no-index\" mode, but\nI doubt it is a good idea to tell what to exclude the \"--no-index\"\nmode in a completely different way.\n\nThe last time I looked at it, I got an impression that the command\nline argument parsing of \"git diff --no-index\" was messy (which is\nsort of inevitable, since unlike the normal \"git diff\", it can\ncompare more than just two \"collections\"---it can take two paths to\nregular files, for example, and in such a case pathspec arguments\ncan play no role), so teaching it pathspec parsing might be a bit of\nwork, though.\n\nThanks for starting an interesting topic.\n"},{"id":"518147","messageId":"CA+P7+xqg3S0q=n3nrTUJJuYicooDm83Q32AkpzRt1u7rH3n3Pw@mail.gmail.com","threadId":"63461","inReplyTo":"xmqqzffe7vbh.fsf@gitster.g","subject":"Re: [PATCH RFC] diff --no-index: teach option to exclude files by pattern","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2025-05-15T16:27:45Z","receivedAt":"2025-05-15T16:27:58Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Wed, May 14, 2025 at 2:10 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Jacob Keller <jacob.e.keller@intel.com> writes:\n>\n> > From: Jacob Keller <jacob.keller@gmail.com>\n> >\n> > Teach git diff --no-index the ability to exclude files by wildmatch\n> > pattern when recursing through directories. The '--exclude' option\n> > builds up a string list containing the patterns. These are checked with\n> > wildmatch() in the read_directory_contents function. If any pattern\n> > matches, then the file is not included in the directory contents.\n>\n> A quite natural question that comes to mind is:\n>\n>     How would we do this for the normal \"git diff\" that is not the\n>     bolted on '--no-index' mode?\n>\n> but ...\n>\n> > The --exclude option is only supported by the --no-index mode. Standard\n> > diff modes support negative pathspecs which is more powerful. I tried to\n> > see if there was a way to add support for negative pathspecs themselves,\n> > but haven't yet figured out if this is possible.\n>\n> ... of course you have thought about it already.  I do agree with\n> you that we should figure out how and teach this mode to also use\n> pathspec, not necessarily only the negative ones but positive ones.\n>\n\nSure, though I think we might need either an option or some other way\nto distinguish pathspec vs the existing non-pathspec mode.\n\n> After all,\n>\n>     $ git diff --no-index [<option>...] dirA dirB\n>\n> is like running\n>\n>     $ diff -r [<option>...] dirA dirB\n>\n> after preparing these two directories like so:\n>\n>     $ git archive revA | ( mkdir dirA && tar Cxf dirA - )\n>     $ git archive revB | ( mkdir dirB && tar Cxf dirB - )\n>\n> Hence it is natural for users to expect that anything you can do\n> with\n>\n>     $ git diff revA revB\n>\n> should be doable, in\n>\n>     $ git diff --no-index dirA dirB\n>\n> and vice versa.  And as you said, when comparing two revisions,\n> you'd use pathspec for this kind of thing.\n>\n>     $ git diff revA revB -- Documentation/ t/ ':!po/'\n>\n> So, I pretty much agree with the need to be able to exclude some\n> parts of the tree(s) from comparison in \"diff --no-index\" mode, but\n> I doubt it is a good idea to tell what to exclude the \"--no-index\"\n> mode in a completely different way.\n>\n\nRight. My main issue was that pathspec seemed to have a bunch of stuff\nbaked into assuming it has a repository.\n\n> The last time I looked at it, I got an impression that the command\n> line argument parsing of \"git diff --no-index\" was messy (which is\n> sort of inevitable, since unlike the normal \"git diff\", it can\n> compare more than just two \"collections\"---it can take two paths to\n> regular files, for example, and in such a case pathspec arguments\n> can play no role), so teaching it pathspec parsing might be a bit of\n> work, though.\n>\n\nRight. It currently requires finding two paths to compare, and some\nDWIM logic to make directory and file comparisons work.\n\npathspec capability is about specifying which things to include or\nexclude from a given search. Hmm..\n\nActually, I think I have a path forward:\n\nwe teach git diff --no-index to treat the first 2 arguments as they\nare now: pointers to the things to compare.\n\nWe check if either or both of those are directories. If they aren't,\nthen additional arguments won't be accepted.\n\nIf we have at least one directory, then instead of rejecting commands\nwith >2 arguments, we interpret any remaining arguments as pathspecs,\nwhich apply to any directory path provided. These can limit the search\nwhen scanning through a directory, so both positive and negative ones\nwould apply in the same say.\n\nI guess the one weirdness is that pathspecs must come after the first\n2 arguments, since we need to find 2 paths first. But this matches the\nway that treeish must come first in git diff-tree -r takes treeish and\nthen pathspecs, and you can't re-order them arbitrarily either.\n\nDoes this sound like a reasonable extension to the existing 2 argument\nform of git diff --no-index?\n\n> Thanks for starting an interesting topic.\n"},{"id":"518159","messageId":"xmqqtt5lzqxh.fsf@gitster.g","threadId":"63461","inReplyTo":"CA+P7+xqg3S0q=n3nrTUJJuYicooDm83Q32AkpzRt1u7rH3n3Pw@mail.gmail.com","subject":"Re: [PATCH RFC] diff --no-index: teach option to exclude files by pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-15T18:09:46Z","receivedAt":"2025-05-15T18:09:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.keller@gmail.com> writes:\n\n> I guess the one weirdness is that pathspecs must come after the first\n> 2 arguments, since we need to find 2 paths first. But this matches the\n> way that treeish must come first in git diff-tree -r takes treeish and\n> then pathspecs, and you can't re-order them arbitrarily either.\n>\n> Does this sound like a reasonable extension to the existing 2 argument\n> form of git diff --no-index?\n\nAbsolutely.\n\nOr you could even use \"--\" convention in the examples you would\nwrite in the documentation, even though you may not absolutely need\nit for the purpose of parsing the command line, to highlight the\nfact that two things to be compared is given and then with an\noptional pathspec after the two things, e.g.,\n\n $ git diff --no-index git-1.6.0 git-2.43.0 -- Documentation/\n\nor something silly like that.\n\n"},{"id":"518180","messageId":"b7fda1fb-3d4e-4115-bca5-63f2e7829ee6@intel.com","threadId":"63461","inReplyTo":"xmqqtt5lzqxh.fsf@gitster.g","subject":"Re: [PATCH RFC] diff --no-index: teach option to exclude files by pattern","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-15T20:24:44Z","receivedAt":"2025-05-15T20:24:48Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 5/15/2025 11:09 AM, Junio C Hamano wrote:\n> Jacob Keller <jacob.keller@gmail.com> writes:\n> \n>> I guess the one weirdness is that pathspecs must come after the first\n>> 2 arguments, since we need to find 2 paths first. But this matches the\n>> way that treeish must come first in git diff-tree -r takes treeish and\n>> then pathspecs, and you can't re-order them arbitrarily either.\n>>\n>> Does this sound like a reasonable extension to the existing 2 argument\n>> form of git diff --no-index?\n> \n> Absolutely.\n> \n> Or you could even use \"--\" convention in the examples you would\n> write in the documentation, even though you may not absolutely need\n> it for the purpose of parsing the command line, to highlight the\n> fact that two things to be compared is given and then with an\n> optional pathspec after the two things, e.g.,\n> \n>  $ git diff --no-index git-1.6.0 git-2.43.0 -- Documentation/\n> \n> or something silly like that.\n> \n\nYea, I'll do that once I get a version with doc. I sent a v2 that works\nok, but I think I need some feedback before I fully polish it, since\nthere are a couple of hacks to get things working.\n\nThanks for the feedback!\n"}]}