{"thread":{"id":"63483","subject":"[PATCH v3 0/4] diff: add pathspec support to --no-index","startedAt":"2025-05-20T00:01:32Z","lastAt":"2025-05-21T23:07:18Z","messageCount":15,"participants":["Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":4},"messages":[{"id":"518457","messageId":"20250520000125.2162144-1-jacob.e.keller@intel.com","threadId":"63483","inReplyTo":null,"subject":"[PATCH v3 0/4] diff: add pathspec support to --no-index","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T00:01:21Z","receivedAt":"2025-05-20T00:01:32Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nThis series adds support for using pathspecs to limit the comparison when\nusing git diff --no-index. This is similar to how you can limit what is\nincluded with pathspecs when comparing inside a repository.\n\nThis likely still needs some reworks, and I have some open questions in the\nfinal implementation:\n\n 1) pathspecs must all come after the first two path arguments, you\n can't re-arrange them to come first. I'm treating them sort of like\n the treeish arguments to git diff-tree.\n\n 2) The pathspecs are interpreted relative to the provided paths, and\n thus will always need to be specified as relative paths, and will be\n interpreted as relative to the root of the search for each path\n separately.\n\n 3) negative pathspecs have to be fully qualified from the root, i.e.\n ':(exclude)file' will only exclude 'a/file' and not 'a/b/file'\n unless you also use '(glob)' or similar. I think this matches the\n other pathspec support, but I an not 100% sure.\n\n 4) I'm not certain about exposing match_pathspec_with_flags as-is,\n since DO_MATCH_EXCLUDE shouldn't be passed. I got the behavior I\n expected with DO_MATCH_LEADING_PATHSPEC, but it feels a bit of a\n weird API. Perhaps match_pathspec could set both flags when is_dir\n is true? But would that break other callers?\n\nHowever, this version now has documentation and some test cases. I found a\nfew issues with my original implementation in v2, which I've fixed.\n\nI also an open to suggestions on better ways to handle the matching.\nCurrently I need a separate set of pathspecs for both paths, since I need to\nmake sure they get rooted appropriately. I don't know if there is a better\nsolution that would allow using the same pathspec structure for both\ncomparisons.\n\nJacob Keller (4):\n prefix_path: support prefixes not ending in trailing slash\n pathspec: expose match_pathspec_with_flags\n pathspec: add flag to indicate operation without repository\n diff --no-index: support limiting by pathspec\n\n pathspec.h | 13 +++++\n builtin/diff.c | 2 +-\n diff-no-index.c | 91 ++++++++++++++++++++++++------\n dir.c | 20 ++++---\n pathspec.c | 6 +-\n setup.c | 4 +-\n Documentation/git-diff.adoc | 10 +++-\n t/t0060-path-utils.sh | 18 ++++++\n t/t4053-diff-no-index.sh | 107 ++++++++++++++++++++++++++++++++++++\n 9 files changed, 240 insertions(+), 31 deletions(-)\n\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"518458","messageId":"20250520000125.2162144-2-jacob.e.keller@intel.com","threadId":"63483","inReplyTo":"20250520000125.2162144-1-jacob.e.keller@intel.com","subject":"[PATCH v3 1/4] prefix_path: support prefixes not ending in trailing slash","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T00:01:22Z","receivedAt":"2025-05-20T00:01:35Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nThe prefix_path_gently() -- and its wrapper prefix_path() -- function\nnormalizes the provided path and optionally adds the provided prefix to\nrelative paths. If the prefix does not end in a trailing slash, the\nfunction will combine the last component of the prefix with the first\ncomponent of the relative path. This is unlikely to produce a desirable\nresult.\n\nTeach prefix_path_gently() to check if the prefix ends in a slash. If it\ndoes not, then insert a slash between the prefix and the path. Take care\nto avoid inserting a slash if the prefix is empty.\n\nAdd test cases to cover the relative path behavior, which was previously\nuntested.\n\nSigned-off-by: Jacob Keller \n---\n setup.c | 4 +++-\n t/t0060-path-utils.sh | 18 ++++++++++++++++++\n 2 files changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/setup.c b/setup.c\nindex f93bd6a24a5d..bd2888500817 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -139,7 +139,9 @@ char *prefix_path_gently(const char *prefix, int len,\n \t\t\treturn NULL;\n \t\t}\n \t} else {\n-\t\tsanitized = xstrfmt(\"%.*s%s\", len, len ? prefix : \"\", path);\n+\t\tsanitized = xstrfmt(\"%.*s%s%s\", len, len ? prefix : \"\",\n+\t\t\t\t !len || prefix[len - 1] == '/' ? \"\" : \"/\",\n+\t\t\t\t path);\n \t\tif (remaining_prefix)\n \t\t\t*remaining_prefix = len;\n \t\tif (normalize_path_copy_len(sanitized, sanitized, remaining_prefix)) {\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 8545cdfab559..9274713aea0c 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -307,6 +307,24 @@ test_expect_success SYMLINKS 'prefix_path works with absolute path to a symlink\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'prefix_path works with relative path' '\n+\techo \"prefix/a\" >expect &&\n+\ttest-tool path-utils prefix_path \"prefix/\" \"a\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'prefix_path works with relative path and prefix not ending in /' '\n+\techo \"prefix/a\" >expect &&\n+\ttest-tool path-utils prefix_path \"prefix\" \"a\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'prefix_path works with relative path and empty prefix' '\n+\techo \"a\" >expect &&\n+\ttest-tool path-utils prefix_path \"\" \"a\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n relative_path /foo/a/b/c/\t/foo/a/b/\tc/\n relative_path /foo/a/b/c/\t/foo/a/b\tc/\n relative_path /foo/a//b//c/\t///foo/a/b//\tc/\t\tPOSIX\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"518459","messageId":"20250520000125.2162144-3-jacob.e.keller@intel.com","threadId":"63483","inReplyTo":"20250520000125.2162144-1-jacob.e.keller@intel.com","subject":"[PATCH v3 2/4] pathspec: expose match_pathspec_with_flags","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T00:01:23Z","receivedAt":"2025-05-20T00:01:35Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nThe do_match_pathspec() function has the DO_MATCH_LEADING_PATHSPEC\noption to allow pathspecs to match when matching \"src\" against a\npathspec like \"src/path/...\". This support is not exposed by\nmatch_pathspec, and the internal flags to do_match_pathspec are not\nexposed outside of dir.c\n\nMake match_pathspec_with_flags public, and expose the\nDO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY flags. The\nDO_MATCH_EXCLUDE flag is kept private in dir.c\n\nThis will be used in a an extension to support pathspec matching in git\ndiff --no-index.\n\nSigned-off-by: Jacob Keller \n---\n pathspec.h | 8 ++++++++\n dir.c | 11 +++++------\n 2 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/pathspec.h b/pathspec.h\nindex de537cff3cb6..d22d4e80248d 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -184,6 +184,14 @@ int match_pathspec(struct index_state *istate,\n \t\t const char *name, int namelen,\n \t\t int prefix, char *seen, int is_dir);\n \n+#define DO_MATCH_DIRECTORY (1<<1)\n+#define DO_MATCH_LEADING_PATHSPEC (1<<2)\n+\n+int match_pathspec_with_flags(struct index_state *istate,\n+\t\t\t const struct pathspec *ps,\n+\t\t\t const char *name, int namelen,\n+\t\t\t int prefix, char *seen, unsigned flags);\n+\n /*\n * Determine whether a pathspec will match only entire index entries (non-sparse\n * files and/or entire sparse directories). If the pathspec has the potential to\ndiff --git a/dir.c b/dir.c\nindex a374972b6243..2f2b654b0252 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -329,9 +329,8 @@ static int do_read_blob(const struct object_id *oid, struct oid_stat *oid_stat,\n \treturn 1;\n }\n \n+// DO_MATCH_EXCLUDE is not public\n #define DO_MATCH_EXCLUDE (1<<0)\n-#define DO_MATCH_DIRECTORY (1<<1)\n-#define DO_MATCH_LEADING_PATHSPEC (1<<2)\n \n /*\n * Does the given pathspec match the given name? A match is found if\n@@ -551,10 +550,10 @@ static int do_match_pathspec(struct index_state *istate,\n \treturn retval;\n }\n \n-static int match_pathspec_with_flags(struct index_state *istate,\n-\t\t\t\t const struct pathspec *ps,\n-\t\t\t\t const char *name, int namelen,\n-\t\t\t\t int prefix, char *seen, unsigned flags)\n+int match_pathspec_with_flags(struct index_state *istate,\n+\t\t\t const struct pathspec *ps,\n+\t\t\t const char *name, int namelen,\n+\t\t\t int prefix, char *seen, unsigned flags)\n {\n \tint positive, negative;\n \tpositive = do_match_pathspec(istate, ps, name, namelen,\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"518460","messageId":"20250520000125.2162144-4-jacob.e.keller@intel.com","threadId":"63483","inReplyTo":"20250520000125.2162144-1-jacob.e.keller@intel.com","subject":"[PATCH v3 3/4] pathspec: add flag to indicate operation without repository","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T00:01:24Z","receivedAt":"2025-05-20T00:01:37Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nA following change will add support for pathspecs to the git diff\n--no-index command. This mode of git diff does not load any repository.\n\nAdd a new PATHSPEC_NO_REPOSITORY flag indicating that we're parsing\npathspecs without a repository.\n\nBoth PATHSPEC_ATTR and PATHSPEC_FROMTOP require a repository to\nfunction. Thus, verify that both of these are set in magic_mask to\nensure they won't be accepted when PATHSPEC_NO_REPOSITORY is set.\n\nCheck PATHSPEC_NO_REPOSITORY when warning about paths outside the\ndirectory tree. When the flag is set, do not look for a git repository\nwhen generating the warning message.\n\nFinally, add a BUG in match_pathspec_item if the istate is NULL but the\npathspec has PATHSPEC_ATTR set. Callers which support PATHSPEC_ATTR\nshould always pass a valid istate, and callers which don't pass a valid\nistate should have set PATHSPEC_ATTR in the magic_mask field to disable\nsupport for attribute-based pathspecs.\n\nSigned-off-by: Jacob Keller \n---\n pathspec.h | 5 +++++\n dir.c | 9 ++++++---\n pathspec.c | 6 +++++-\n 3 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git a/pathspec.h b/pathspec.h\nindex d22d4e80248d..2b47f9a2a213 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -76,6 +76,11 @@ struct pathspec {\n * allowed, then it will automatically set for every pathspec.\n */\n #define PATHSPEC_LITERAL_PATH (1<<6)\n+/*\n+ * For git diff --no-index, indicate that we are operating without\n+ * a repository or index.\n+ */\n+#define PATHSPEC_NO_REPOSITORY (1<<7)\n \n /**\n * Given command line arguments and a prefix, convert the input to\ndiff --git a/dir.c b/dir.c\nindex 2f2b654b0252..45aac0bfacab 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -396,9 +396,12 @@ static int match_pathspec_item(struct index_state *istate,\n \t strncmp(item->match, name - prefix, item->prefix))\n \t\treturn 0;\n \n-\tif (item->attr_match_nr &&\n-\t !match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n-\t\treturn 0;\n+\tif (item->attr_match_nr) {\n+\t\tif (!istate)\n+\t\t\tBUG(\"magic PATHSPEC_ATTR requires an index\");\n+\t\tif (!match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n+\t\t\treturn 0;\n+\t}\n \n \t/* If the match was just the prefix, we matched */\n \tif (!*match)\ndiff --git a/pathspec.c b/pathspec.c\nindex 2b4e434bc0aa..a3ddd701c740 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -492,7 +492,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n \t\tif (!match) {\n \t\t\tconst char *hint_path;\n \n-\t\t\tif (!have_git_dir())\n+\t\t\tif ((flags & PATHSPEC_NO_REPOSITORY) || !have_git_dir())\n \t\t\t\tdie(_(\"'%s' is outside the directory tree\"),\n \t\t\t\t copyfrom);\n \t\t\thint_path = repo_get_work_tree(the_repository);\n@@ -614,6 +614,10 @@ void parse_pathspec(struct pathspec *pathspec,\n \t (flags & PATHSPEC_PREFER_FULL))\n \t\tBUG(\"PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL are incompatible\");\n \n+\tif ((flags & PATHSPEC_NO_REPOSITORY) &&\n+\t (~magic_mask & (PATHSPEC_ATTR | PATHSPEC_FROMTOP)))\n+\t\tBUG(\"PATHSPEC_NO_REPOSITORY is incompatible with PATHSPEC_ATTR and PATHSPEC_FROMTOP\");\n+\n \t/* No arguments with prefix -> prefix pathspec */\n \tif (!entry) {\n \t\tif (flags & PATHSPEC_PREFER_FULL)\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"518461","messageId":"20250520000125.2162144-5-jacob.e.keller@intel.com","threadId":"63483","inReplyTo":"20250520000125.2162144-1-jacob.e.keller@intel.com","subject":"[PATCH v3 4/4] diff --no-index: support limiting by pathspec","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T00:01:25Z","receivedAt":"2025-05-20T00:01:37Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller \n\nThe --no-index option of git-diff enables using the diff machinery from\ngit while operating outside of a repository. This mode of git diff is\nable to compare directories and produce a diff of their contents.\n\nWhen operating git diff in a repository, git has the notion of\n\"pathspecs\" which can specify which files to compare. In particular,\nwhen using git to diff two trees, you might invoke:\n\n $ git diff-tree -r .\n\nwhere the treeish could point to a subdirectory of the repository.\n\nWhen invoked this way, users can limit the selected paths of the tree by\nusing a pathspec. Either by providing some list of paths to accept, or\nby removing paths via a negative refspec.\n\nThe git diff --no-index mode does not support pathspecs, and cannot\nlimit the diff output in this way. Other diff programs such as GNU\ndifftools have options for excluding paths based on a pattern match.\nHowever, using git diff as a diff replacement has several advantages\nover many popular diff tools, including coloring moved lines, rename\ndetections, and similar.\n\nTeach git diff --no-index how to handle pathspecs to limit the\ncomparisons. This will only be supported if both provided paths are\ndirectories.\n\nFor comparisons where one path isn't a directory, the --no-index mode\nalready has some DWIM shortcuts implemented in the fixup_paths()\nfunction.\n\nModify the fixup_paths function to return 1 if both paths are\ndirectories. If this is the case, interpret any extra arguments to git\ndiff as pathspecs via parse_pathspec. Add a new PATHSPEC_NO_REPOSITORY\nflag to indicate to the parser that we do not have repository. Use this\nto aid in error message reporting in the event that the user happens to\ninvoke git diff --no-index from a repository.\n\nUse parse_pathspec to load the remaining arguments (if any) to git diff\n--no-index as pathspec items. Disable PATHSPEC_ATTR support since we do\nnot have a repository to do attribute lookup. Disable PATHSPEC_FROMTOP\nsince we do not have a repository root. All pathspecs are treated as\nrooted at the provided comparison paths.\n\nLoad the pathspecs twice, once prefixed with paths[0] and once prefixed\nwith paths[1]. I considered trying to avoid this, but don't yet have a\nworkable solution that reuses the same pathspec objects. We need the\ndirectory paths as prefixes otherwise the match_pathspec won't compare\nproperly.\n\nPass the pathspec object for both paths into queue_diff, who in-turn\npasses these along to read_directory_contents.\n\nModify read_directory_contents to check against the pathspecs when\nscanning the directory. In order to properly recurse, we need to ensure\nthat directories match, and that we continue iterating down the nested\ndirectory structure:\n\n $ git diff --no-index a b c/d\n\nThis should include all paths in a and b which match the c/d pathspec.\nIn particular, if there was 'a/c/d' we need to match it. But to check\n'a/c/d', we need to first get to that part, which requires comparing\n'a/c' first. With the match_pathspec() function, 'a/c' won't match the\n'a/c/d' pathspec string.\n\nHowever, if we always passed DO_MATCH_LEADING_PATHSPEC, then we will\nincorrectly match in certain cases such as matching 'a/c' against\n':(glob)**/d'. The match logic will see that a matches the leading part\nof the **/ and accept this even tho c doesn't match. The trick seems to\nbe setting both DO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY when\nchecking directories, but set neither of them when checking files.\n\nSome other gotchas and open questions:\n\n 1) pathspecs must all come after the first two path arguments, you\n can't re-arrange them to come first. I'm treating them sort of like\n the treeish arguments to git diff-tree.\n\n 2) The pathspecs are interpreted relative to the provided paths, and\n thus will always need to be specified as relative paths, and will be\n interpreted as relative to the root of the search for each path\n separately.\n\n 3) negative pathspecs have to be fully qualified from the root, i.e.\n ':(exclude)file' will only exclude 'a/file' and not 'a/b/file'\n unless you also use '(glob)' or similar. I think this matches the\n other pathspec support, but I an not 100% sure.\n\n 4) I'm not certain about exposing match_pathspec_with_flags as-is,\n since DO_MATCH_EXCLUDE shouldn't be passed. I got the behavior I\n expected with DO_MATCH_LEADING_PATHSPEC, but it feels a bit of a\n weird API. Perhaps match_pathspec could set both flags when is_dir\n is true? But would that break other callers?\n\nSigned-off-by: Jacob Keller \n---\n builtin/diff.c | 2 +-\n diff-no-index.c | 91 ++++++++++++++++++++++++------\n Documentation/git-diff.adoc | 10 +++-\n t/t4053-diff-no-index.sh | 107 ++++++++++++++++++++++++++++++++++++\n 4 files changed, 190 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex fa963808c318..c6231edce4e8 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -35,7 +35,7 @@ static const char builtin_diff_usage[] =\n \" or: git diff [] [--merge-base] [...] [--] [...]\\n\"\n \" or: git diff [] ... [--] [...]\\n\"\n \" or: git diff [] \\n\"\n-\" or: git diff [] --no-index [--] \"\n+\" or: git diff [] --no-index [--] [...]\"\n \"\\n\"\n COMMON_DIFF_OPTIONS_HELP;\n \ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 9739b2b268b9..e9e8006487c4 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -15,20 +15,47 @@\n #include \"gettext.h\"\n #include \"revision.h\"\n #include \"parse-options.h\"\n+#include \"pathspec.h\"\n #include \"string-list.h\"\n #include \"dir.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 const struct pathspec *pathspec)\n {\n+\tstruct strbuf match = STRBUF_INIT;\n+\tint len;\n \tDIR *dir;\n \tstruct dirent *e;\n \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-\t\tstring_list_insert(list, e->d_name);\n+\tif (pathspec) {\n+\t\tstrbuf_addstr(&match, path);\n+\t\tstrbuf_complete(&match, '/');\n+\t\tlen = match.len;\n+\t}\n \n+\twhile ((e = readdir_skip_dot_and_dotdot(dir))) {\n+\t\tif (pathspec) {\n+\t\t\tint flags = 0;\n+\n+\t\t\tstrbuf_setlen(&match, len);\n+\t\t\tstrbuf_addstr(&match, e->d_name);\n+\n+\t\t\tif (e->d_type == DT_DIR)\n+\t\t\t\tflags = DO_MATCH_LEADING_PATHSPEC | DO_MATCH_DIRECTORY;\n+\n+\t\t\tif (!match_pathspec_with_flags(NULL, pathspec,\n+\t\t\t\t\t\t match.buf, match.len,\n+\t\t\t\t\t\t 0, NULL, flags))\n+\t\t\t\tcontinue;\n+\t\t}\n+\n+\t\tstring_list_insert(list, e->d_name);\n+\t}\n+\n+\tstrbuf_release(&match);\n \tclosedir(dir);\n \treturn 0;\n }\n@@ -131,7 +158,8 @@ static struct diff_filespec *noindex_filespec(const struct git_hash_algo *algop,\n }\n \n static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,\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 const struct pathspec *ps1, const struct pathspec *ps2)\n {\n \tint mode1 = 0, mode2 = 0;\n \tenum special special1 = SPECIAL_NONE, special2 = SPECIAL_NONE;\n@@ -171,9 +199,9 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,\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, ps1))\n \t\t\treturn -1;\n-\t\tif (name2 && read_directory_contents(name2, &p2)) {\n+\t\tif (name2 && read_directory_contents(name2, &p2, ps2)) {\n \t\t\tstring_list_clear(&p1, 0);\n \t\t\treturn -1;\n \t\t}\n@@ -218,7 +246,7 @@ static int queue_diff(struct diff_options *o, const struct git_hash_algo *algop,\n \t\t\t\tn2 = buffer2.buf;\n \t\t\t}\n \n-\t\t\tret = queue_diff(o, algop, n1, n2, 1);\n+\t\t\tret = queue_diff(o, algop, n1, n2, 1, ps1, ps2);\n \t\t}\n \t\tstring_list_clear(&p1, 0);\n \t\tstring_list_clear(&p2, 0);\n@@ -258,8 +286,10 @@ static void append_basename(struct strbuf *path, const char *dir, const char *fi\n * DWIM \"diff D F\" into \"diff D/F F\" and \"diff F D\" into \"diff F D/F\"\n * Note that we append the basename of F to D/, so \"diff a/b/file D\"\n * becomes \"diff a/b/file D/file\", not \"diff a/b/file D/a/b/file\".\n+ *\n+ * Return 1 if both paths are directories, 0 otherwise.\n */\n-static void fixup_paths(const char **path, struct strbuf *replacement)\n+static int fixup_paths(const char **path, struct strbuf *replacement)\n {\n \tstruct stat st;\n \tunsigned int isdir0 = 0, isdir1 = 0;\n@@ -282,25 +312,30 @@ static void fixup_paths(const char **path, struct strbuf *replacement)\n \tif ((isdir0 && ispipe1) || (ispipe0 && isdir1))\n \t\tdie(_(\"cannot compare a named pipe to a directory\"));\n \n-\tif (isdir0 == isdir1)\n-\t\treturn;\n+\t/* if both paths are directories, we will enable pathspecs */\n+\tif (isdir0 && isdir1)\n+\t\treturn 1;\n+\n \tif (isdir0) {\n \t\tappend_basename(replacement, path[0], path[1]);\n \t\tpath[0] = replacement->buf;\n-\t} else {\n+\t} else if (isdir1) {\n \t\tappend_basename(replacement, path[1], path[0]);\n \t\tpath[1] = replacement->buf;\n \t}\n+\n+\treturn 0;\n }\n \n static const char * const diff_no_index_usage[] = {\n-\tN_(\"git diff --no-index [] \"),\n+\tN_(\"git diff --no-index [] [...]\"),\n \tNULL\n };\n \n int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \t\t int implicit_no_index, int argc, const char **argv)\n {\n+\tstruct pathspec pathspec1, pathspec2, *ps1 = NULL, *ps2 = NULL;\n \tint i, no_index;\n \tint ret = 1;\n \tconst char *paths[2];\n@@ -317,13 +352,12 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \toptions = add_diff_options(no_index_options, &revs->diffopt);\n \targc = parse_options(argc, argv, revs->prefix, options,\n \t\t\t diff_no_index_usage, 0);\n-\tif (argc != 2) {\n+\tif (argc < 2) {\n \t\tif (implicit_no_index)\n \t\t\twarning(_(\"Not a git repository. Use --no-index to \"\n \t\t\t\t \"compare two paths outside a working tree\"));\n \t\tusage_with_options(diff_no_index_usage, options);\n \t}\n-\tFREE_AND_NULL(options);\n \tfor (i = 0; i < 2; i++) {\n \t\tconst char *p = argv[i];\n \t\tif (!strcmp(p, \"-\"))\n@@ -337,7 +371,28 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \t\tpaths[i] = p;\n \t}\n \n-\tfixup_paths(paths, &replacement);\n+\t/* TODO: should we try to catch pathspec-like paths first and warn or\n+\t * error? We accepted those as valid 'paths' before so it seems\n+\t * unlikely we can change that behavior.\n+\t */\n+\tif (fixup_paths(paths, &replacement)) {\n+\t\tparse_pathspec(&pathspec1, PATHSPEC_FROMTOP | PATHSPEC_ATTR,\n+\t\t\t PATHSPEC_PREFER_FULL | PATHSPEC_NO_REPOSITORY,\n+\t\t\t paths[0], &argv[2]);\n+\t\tif (pathspec1.nr)\n+\t\t\tps1 = &pathspec1;\n+\n+\t\tparse_pathspec(&pathspec2, PATHSPEC_FROMTOP | PATHSPEC_ATTR,\n+\t\t\t PATHSPEC_PREFER_FULL | PATHSPEC_NO_REPOSITORY,\n+\t\t\t paths[1], &argv[2]);\n+\t\tif (pathspec2.nr)\n+\t\t\tps2 = &pathspec2;\n+\t} else if (argc > 2) {\n+\t\twarning(_(\"Limiting comparison with pathspecs is only \"\n+\t\t\t \"supported if both paths are directories.\"));\n+\t\tusage_with_options(diff_no_index_usage, options);\n+\t}\n+\tFREE_AND_NULL(options);\n \n \trevs->diffopt.skip_stat_unmatch = 1;\n \tif (!revs->diffopt.output_format)\n@@ -354,7 +409,7 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \tsetup_diff_pager(&revs->diffopt);\n \trevs->diffopt.flags.exit_with_status = 1;\n \n-\tif (queue_diff(&revs->diffopt, algop, paths[0], paths[1], 0))\n+\tif (queue_diff(&revs->diffopt, algop, paths[0], paths[1], 0, ps1, ps2))\n \t\tgoto out;\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"1/\", \"2/\");\n \tdiffcore_std(&revs->diffopt);\n@@ -370,5 +425,9 @@ int diff_no_index(struct rev_info *revs, const struct git_hash_algo *algop,\n \tfor (i = 0; i < ARRAY_SIZE(to_free); i++)\n \t\tfree(to_free[i]);\n \tstrbuf_release(&replacement);\n+\tif (ps1)\n+\t\tclear_pathspec(ps1);\n+\tif (ps2)\n+\t\tclear_pathspec(ps2);\n \treturn ret;\n }\ndiff --git a/Documentation/git-diff.adoc b/Documentation/git-diff.adoc\nindex dec173a34517..272331afbaec 100644\n--- a/Documentation/git-diff.adoc\n+++ b/Documentation/git-diff.adoc\n@@ -14,7 +14,7 @@ git diff [] --cached [--merge-base] [] [--] [...]\n git diff [] [--merge-base] [...] [--] [...]\n git diff [] ... [--] [...]\n git diff [] \n-git diff [] --no-index [--] \n+git diff [] --no-index [--] [...]\n \n DESCRIPTION\n -----------\n@@ -31,14 +31,18 @@ files on disk.\n \tfurther add to the index but you still haven't. You can\n \tstage these changes by using linkgit:git-add[1].\n \n-`git diff [] --no-index [--] `::\n+`git diff [] --no-index [--] [...]`::\n \n \tThis form is to compare the given two paths on the\n \tfilesystem. You can omit the `--no-index` option when\n \trunning the command in a working tree controlled by Git and\n \tat least one of the paths points outside the working tree,\n \tor when running the command outside a working tree\n-\tcontrolled by Git. This form implies `--exit-code`.\n+\tcontrolled by Git. This form implies `--exit-code`. If both\n+\tpaths point to directories, additional pathspecs may be\n+\tprovided. These will limit the files included in the\n+\tdifference. All such pathspecs must be relative as they\n+\tapply to both sides of the diff.\n \n `git diff [] --cached [--merge-base] [] [--] [...]`::\n \ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 5e5bad61ca1e..60437accfd98 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -295,4 +295,111 @@ test_expect_success PIPE,SYMLINKS 'diff --no-index reads from pipes' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'diff --no-index F F rejects pathspecs' '\n+\ttest_must_fail git diff --no-index -- a/1 a/2 a 2>actual.err &&\n+\ttest_grep \"usage: git diff --no-index\" actual.err\n+'\n+\n+test_expect_success 'diff --no-index D F rejects pathspecs' '\n+\ttest_must_fail git diff --no-index -- a a/2 a 2>actual.err &&\n+\ttest_grep \"usage: git diff --no-index\" actual.err\n+'\n+\n+test_expect_success 'diff --no-index F D rejects pathspecs' '\n+\ttest_must_fail git diff --no-index -- a/1 b b 2>actual.err &&\n+\ttest_grep \"usage: git diff --no-index\" actual.err\n+'\n+\n+test_expect_success 'diff --no-index with pathspec' '\n+\ttest_expect_code 1 git diff --no-index a b 1 >actual &&\n+\tcat >expect <<-EOF &&\n+\tdiff --git a/a/1 b/a/1\n+\tdeleted file mode 100644\n+\tindex d00491f..0000000\n+\t--- a/a/1\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-1\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'diff --no-index with pathspec no matches' '\n+\ttest_expect_code 0 git diff --no-index a b missing\n+'\n+\n+test_expect_success 'diff --no-index with negative pathspec' '\n+\ttest_expect_code 1 git diff --no-index a b \":!2\" >actual &&\n+\tcat >expect <<-EOF &&\n+\tdiff --git a/a/1 b/a/1\n+\tdeleted file mode 100644\n+\tindex d00491f..0000000\n+\t--- a/a/1\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-1\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'setup nested' '\n+\tmkdir -p c/1/2 &&\n+\tmkdir -p d/1/2 &&\n+\techo 1 >c/1/2/a &&\n+\techo 2 >c/1/2/b\n+'\n+\n+test_expect_success 'diff --no-index with pathspec nested negative pathspec' '\n+\ttest_expect_code 0 git diff --no-index c d \":!1\"\n+'\n+\n+test_expect_success 'diff --no-index with pathspec nested pathspec' '\n+\ttest_expect_code 1 git diff --no-index c d 1/2 >actual &&\n+\tcat >expect <<-EOF &&\n+\tdiff --git a/c/1/2/a b/c/1/2/a\n+\tdeleted file mode 100644\n+\tindex d00491f..0000000\n+\t--- a/c/1/2/a\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-1\n+\tdiff --git a/c/1/2/b b/c/1/2/b\n+\tdeleted file mode 100644\n+\tindex 0cfbf08..0000000\n+\t--- a/c/1/2/b\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-2\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'diff --no-index with pathspec glob' '\n+\ttest_expect_code 1 git diff --no-index c d \":(glob)**/a\" >actual &&\n+\tcat >expect <<-EOF &&\n+\tdiff --git a/c/1/2/a b/c/1/2/a\n+\tdeleted file mode 100644\n+\tindex d00491f..0000000\n+\t--- a/c/1/2/a\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-1\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'diff --no-index with pathspec glob and exclude' '\n+\ttest_expect_code 1 git diff --no-index c d \":(glob,exclude)**/a\" >actual &&\n+\tcat >expect <<-EOF &&\n+\tdiff --git a/c/1/2/b b/c/1/2/b\n+\tdeleted file mode 100644\n+\tindex 0cfbf08..0000000\n+\t--- a/c/1/2/b\n+\t+++ /dev/null\n+\t@@ -1 +0,0 @@\n+\t-2\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.48.1.397.gec9d649cc640\n\n"},{"id":"518488","messageId":"xmqqh61fl58v.fsf@gitster.g","threadId":"63483","inReplyTo":"20250520000125.2162144-2-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 1/4] prefix_path: support prefixes not ending in trailing slash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-20T14:35:28Z","receivedAt":"2025-05-20T14:35:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> From: Jacob Keller \n>\n> The prefix_path_gently() -- and its wrapper prefix_path() -- function\n> normalizes the provided path and optionally adds the provided prefix to\n> relative paths.\n\nI find \"optionally\" confusing here. The original intended use case\nof this function being that \"the provided path\", which comes from\nthe end user from the command line that names a file relative to the\ndirectory the usre started the \"git\" command in, needs to be\nadjusted after \"git\" chdir's up to the top-level of the working\ntree, and the way to do so is to prepend the \"prefix\" computed by\n\"git\" (which always ends with a slash). Adding the prefix to\nrelative paths is the central part of what it has to do. When the\nend-user supplied path goes up e.g., \"../file\", we may have to lose\na level or more of the prefix before prepending, but that does not\nchange the fact that the helper function is about prepending the\nprefix.\n\nAnd as you can guess from the above description, if the caller\npasses prefix that does not end in a trailing slash, the caller is\nbuggy.\n\n> If the prefix does not end in a trailing slash, the\n> function will combine the last component of the prefix with the first\n> component of the relative path. This is unlikely to produce a desirable\n> result.\n\nTrue, and I do not mind being lenient to buggy callers, but given\nthat the majority of callers (i.e. all the existing callers) not\nbeing buggy, I wonder if it is better to check and append a slash to\nthe end by a new caller that feeds a prefix that directly comes from\nthe end user?\n\nUnlike prefix that is internally generated by Git, we'd need to be a\nlot more careful if we are taking end-user input directly and\npassing it as a \"prefix\" (I take that the possible lack of trailing\nslash as a signal that you are doing something like that in this\nseries). We may need to check and correct that they do not contain\n\"./\", \"//\", or \"../\", for example, anyway, and adding the required\ntrailing slash at the end sounds like something we may want to do as\npart of that input validation and massaging _before_ calling these\nhelper functions.\n"},{"id":"518491","messageId":"xmqq7c2bl51c.fsf@gitster.g","threadId":"63483","inReplyTo":"20250520000125.2162144-3-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 2/4] pathspec: expose match_pathspec_with_flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-20T14:39:59Z","receivedAt":"2025-05-20T14:40:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> From: Jacob Keller \n>\n> The do_match_pathspec() function has the DO_MATCH_LEADING_PATHSPEC\n> option to allow pathspecs to match when matching \"src\" against a\n> pathspec like \"src/path/...\". This support is not exposed by\n> match_pathspec, and the internal flags to do_match_pathspec are not\n> exposed outside of dir.c\n>\n> Make match_pathspec_with_flags public, and expose the\n> DO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY flags. The\n> DO_MATCH_EXCLUDE flag is kept private in dir.c\n>\n> This will be used in a an extension to support pathspec matching in git\n> diff --no-index.\n>\n> Signed-off-by: Jacob Keller \n> ---\n> pathspec.h | 8 ++++++++\n> dir.c | 11 +++++------\n> 2 files changed, 13 insertions(+), 6 deletions(-)\n\nYou use diff.orderfile? Not complaining, just finding it amusing\nthat somebody uses the feature ;-).\n\n> diff --git a/pathspec.h b/pathspec.h\n> index de537cff3cb6..d22d4e80248d 100644\n> --- a/pathspec.h\n> +++ b/pathspec.h\n> @@ -184,6 +184,14 @@ int match_pathspec(struct index_state *istate,\n> \t\t const char *name, int namelen,\n> \t\t int prefix, char *seen, int is_dir);\n> \n> +#define DO_MATCH_DIRECTORY (1<<1)\n> +#define DO_MATCH_LEADING_PATHSPEC (1<<2)\n> +\n> +int match_pathspec_with_flags(struct index_state *istate,\n> +\t\t\t const struct pathspec *ps,\n> +\t\t\t const char *name, int namelen,\n> +\t\t\t int prefix, char *seen, unsigned flags);\n> +\n> /*\n> * Determine whether a pathspec will match only entire index entries (non-sparse\n> * files and/or entire sparse directories). If the pathspec has the potential to\n> diff --git a/dir.c b/dir.c\n> index a374972b6243..2f2b654b0252 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -329,9 +329,8 @@ static int do_read_blob(const struct object_id *oid, struct oid_stat *oid_stat,\n> \treturn 1;\n> }\n> \n> +// DO_MATCH_EXCLUDE is not public\n\nWe do not use // comments (outside borrowed code anyway).\n\n> #define DO_MATCH_EXCLUDE (1<<0)\n> -#define DO_MATCH_DIRECTORY (1<<1)\n> -#define DO_MATCH_LEADING_PATHSPEC (1<<2)\n> \n> /*\n> * Does the given pathspec match the given name? A match is found if\n> @@ -551,10 +550,10 @@ static int do_match_pathspec(struct index_state *istate,\n> \treturn retval;\n> }\n> \n> -static int match_pathspec_with_flags(struct index_state *istate,\n> -\t\t\t\t const struct pathspec *ps,\n> -\t\t\t\t const char *name, int namelen,\n> -\t\t\t\t int prefix, char *seen, unsigned flags)\n> +int match_pathspec_with_flags(struct index_state *istate,\n> +\t\t\t const struct pathspec *ps,\n> +\t\t\t const char *name, int namelen,\n> +\t\t\t int prefix, char *seen, unsigned flags)\n> {\n> \tint positive, negative;\n> \tpositive = do_match_pathspec(istate, ps, name, namelen,\n"},{"id":"518499","messageId":"xmqqwmabjoww.fsf@gitster.g","threadId":"63483","inReplyTo":"20250520000125.2162144-4-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 3/4] pathspec: add flag to indicate operation without repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-20T15:13:35Z","receivedAt":"2025-05-20T15:13:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> From: Jacob Keller \n>\n> A following change will add support for pathspecs to the git diff\n> --no-index command. This mode of git diff does not load any repository.\n>\n> Add a new PATHSPEC_NO_REPOSITORY flag indicating that we're parsing\n> pathspecs without a repository.\n>\n> Both PATHSPEC_ATTR and PATHSPEC_FROMTOP require a repository to\n> function. Thus, verify that both of these are set in magic_mask to\n> ensure they won't be accepted when PATHSPEC_NO_REPOSITORY is set.\n>\n> Check PATHSPEC_NO_REPOSITORY when warning about paths outside the\n> directory tree. When the flag is set, do not look for a git repository\n> when generating the warning message.\n>\n> Finally, add a BUG in match_pathspec_item if the istate is NULL but the\n> pathspec has PATHSPEC_ATTR set. Callers which support PATHSPEC_ATTR\n> should always pass a valid istate, and callers which don't pass a valid\n> istate should have set PATHSPEC_ATTR in the magic_mask field to disable\n> support for attribute-based pathspecs.\n\nAll very sensible considerations.\n\n> diff --git a/dir.c b/dir.c\n> index 2f2b654b0252..45aac0bfacab 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -396,9 +396,12 @@ static int match_pathspec_item(struct index_state *istate,\n> \t strncmp(item->match, name - prefix, item->prefix))\n> \t\treturn 0;\n> \n> -\tif (item->attr_match_nr &&\n> -\t !match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n> -\t\treturn 0;\n> +\tif (item->attr_match_nr) {\n> +\t\tif (!istate)\n> +\t\t\tBUG(\"magic PATHSPEC_ATTR requires an index\");\n> +\t\tif (!match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n> +\t\t\treturn 0;\n> +\t}\n\nIt is a bit curious why we do not check PATHSPEC_NO_REPOSITORY here,\nbut it is OK, because it is a BUG for istate to be NULL when we have\na repository anyway.\n\n> diff --git a/pathspec.c b/pathspec.c\n> index 2b4e434bc0aa..a3ddd701c740 100644\n> --- a/pathspec.c\n> +++ b/pathspec.c\n> @@ -492,7 +492,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n> \t\tif (!match) {\n> \t\t\tconst char *hint_path;\n> \n> -\t\t\tif (!have_git_dir())\n> +\t\t\tif ((flags & PATHSPEC_NO_REPOSITORY) || !have_git_dir())\n> \t\t\t\tdie(_(\"'%s' is outside the directory tree\"),\n> \t\t\t\t copyfrom);\n> \t\t\thint_path = repo_get_work_tree(the_repository);\n\nThis is a part of generating an error message. We die early to\navoid having to call get-work-tree when we know we are not even in\nany working tree, which makes sense.\n\n> @@ -614,6 +614,10 @@ void parse_pathspec(struct pathspec *pathspec,\n> \t (flags & PATHSPEC_PREFER_FULL))\n> \t\tBUG(\"PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL are incompatible\");\n> \n> +\tif ((flags & PATHSPEC_NO_REPOSITORY) &&\n> +\t (~magic_mask & (PATHSPEC_ATTR | PATHSPEC_FROMTOP)))\n> +\t\tBUG(\"PATHSPEC_NO_REPOSITORY is incompatible with PATHSPEC_ATTR and PATHSPEC_FROMTOP\");\n\nHmph, I am not sure if this change is correct. The magic_mask\nparameter is passed by a caller to say \"even if parsr_pathspec()\nparses a pathspec using a certain set of features properly, the\ncaller is not prepared to handle the parsed result\". If magic_mask\nlacks PATHSPEC_ATTR, that does not necessarily mean that the given\npathspec contains any pathspec items that do use the attr magic. It\nmerely says that the caller is not prepared to handle a pathspec\nitem that uses the attr magic feature.\n\nIf we are going to add a call to parse_pathspec() in a code path\nthat is specific to diff-no-index, isn't it sufficient to pass\nPATHSPEC_ATTR and PATHSPEC_FROMTOP as magic_mask without this\nchange?\n\n\n"},{"id":"518503","messageId":"xmqqy0uri6st.fsf@gitster.g","threadId":"63483","inReplyTo":"20250520000125.2162144-5-jacob.e.keller@intel.com","subject":"Re: [PATCH v3 4/4] diff --no-index: support limiting by pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-20T16:30:10Z","receivedAt":"2025-05-20T16:30:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller writes:\n\n> The git diff --no-index mode does not support pathspecs, and cannot\n> limit the diff output in this way. Other diff programs such as GNU\n> difftools have options for excluding paths based on a pattern match.\n\nTrue.\n\n> However, using git diff as a diff replacement has several advantages\n> over many popular diff tools, including coloring moved lines, rename\n> detections, and similar.\n\nI said this when we introduced \"--no-index\", but back when \"git\" was\nstill young, it would have helped a lot wider developer populations\nif we didn't do \"git diff --no-index\" and instead donated these\nfeatures to \"many popular diff tools\". We however chose to be lazy\nand selfish---those who want to use these features are better off\ninstalling \"git\" even if they are not using it for version control,\nonly to use it as \"better diff\".\n\nPeople are still welcome to add \"coloring moved lines, rename\ndetections and similar\" to \"many popular diff tools\", but given that\n\"git\" has become fairly popular and widely used, it may not matter\nall that much any more that we were lazy and selfish ;-).\n\n> Teach git diff --no-index how to handle pathspecs to limit the\n> comparisons. This will only be supported if both provided paths are\n> directories.\n\nGood. If you are giving only two files, pathspec limiting would not\nmake much sense. If you are giving a file and a directory, lazily\ngive only F and D to compare F with D/F, that is essentially giving\nonly two files F and D/F, so pathspec limiting would not make much\nsense, either. pathspec limited comparison would make sense only\nwhen you are talking about two sets of files.\n\n> However, if we always passed DO_MATCH_LEADING_PATHSPEC, then we will\n> incorrectly match in certain cases such as matching 'a/c' against\n> ':(glob)**/d'. The match logic will see that a matches the leading part\n> of the **/ and accept this even tho c doesn't match. The trick seems to\n> be setting both DO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY when\n> checking directories, but set neither of them when checking files.\n\nSounds sensible.\n\n> Some other gotchas and open questions:\n>\n> 1) pathspecs must all come after the first two path arguments, you\n> can't re-arrange them to come first. I'm treating them sort of like\n> the treeish arguments to git diff-tree.\n\nExactly. \"git diff\" proper is about comparing two sets of files,\neither comparing two tree-ishes \"git diff master next\", comparing a\ntree-ish and the index \"git diff --cached HEAD\", comparing a\ntree-ish and the working tree files \"git diff HEAD\", or comparing\nthe index and the working tree files \"git diff\". It is a natural\nextension that \"git --no-index dirA dirB\" compares contents of the\ntwo directories. In all of these forms, it is common that the\ncomparison can be pathspec limited by giving pathspec elements as\nthe command line arguments at the end.\n\n> 2) The pathspecs are interpreted relative to the provided paths, and\n> thus will always need to be specified as relative paths, and will be\n> interpreted as relative to the root of the search for each path\n> separately.\n\nYes, that is not anything new or something we need to point out as\nif it is any different from the \"normal\" pathspec.\n\n> 3) negative pathspecs have to be fully qualified from the root, i.e.\n> ':(exclude)file' will only exclude 'a/file' and not 'a/b/file'\n> unless you also use '(glob)' or similar. I think this matches the\n> other pathspec support, but I an not 100% sure.\n\nI think that is correct \"git ls-files :(exclude)po\" does not exclude\ngit-gui/po, for example.\n\n> -`git diff [] --no-index [--] `::\n> +`git diff [] --no-index [--] [...]`::\n\nThis is a bit unfortunate. The disambiguating \"--\" should ideally\nbe between the \"things to be compared\" and the pathspec, as the\nformer corresponds to in the normal \"git diff\" invocation.\n\n> +\t... If both\n> +\tpaths point to directories, additional pathspecs may be\n> +\tprovided. These will limit the files included in the\n> +\tdifference. All such pathspecs must be relative as they\n> +\tapply to both sides of the diff.\n\n\"as they\" -> \"and they\"?\n\n> +test_expect_success 'diff --no-index with pathspec' '\n> +\ttest_expect_code 1 git diff --no-index a b 1 >actual &&\n> +\tcat >expect <<-EOF &&\n> +\tdiff --git a/a/1 b/a/1\n> +\tdeleted file mode 100644\n> +\tindex d00491f..0000000\n> +\t--- a/a/1\n> +\t+++ /dev/null\n> +\t@@ -1 +0,0 @@\n> +\t-1\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nIf you use --name-only or --name-status would the test become\nsimpler?\n\n> +\n> +test_expect_success 'diff --no-index with pathspec no matches' '\n> +\ttest_expect_code 0 git diff --no-index a b missing\n> +'\n\nOK.\n\n> +test_expect_success 'diff --no-index with negative pathspec' '\n> +\ttest_expect_code 1 git diff --no-index a b \":!2\" >actual &&\n> +\tcat >expect <<-EOF &&\n> +\tdiff --git a/a/1 b/a/1\n> +\tdeleted file mode 100644\n> +\tindex d00491f..0000000\n> +\t--- a/a/1\n> +\t+++ /dev/null\n> +\t@@ -1 +0,0 @@\n> +\t-1\n> +\tEOF\n> +\ttest_cmp expect actual\n> +'\n\nOK.\n\nAll other tests also look sensible.\n\nThanks.\n"},{"id":"518537","messageId":"6e94605a-6706-4374-baeb-7e87d27f1bf9@intel.com","threadId":"63483","inReplyTo":"xmqqh61fl58v.fsf@gitster.g","subject":"Re: [PATCH v3 1/4] prefix_path: support prefixes not ending in trailing slash","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T22:34:48Z","receivedAt":"2025-05-20T22:34:54Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 5/20/2025 7:35 AM, Junio C Hamano wrote:\n> Jacob Keller writes:\n> \n>> From: Jacob Keller \n>>\n>> The prefix_path_gently() -- and its wrapper prefix_path() -- function\n>> normalizes the provided path and optionally adds the provided prefix to\n>> relative paths.\n> \n> I find \"optionally\" confusing here. The original intended use case\n> of this function being that \"the provided path\", which comes from\n> the end user from the command line that names a file relative to the\n> directory the usre started the \"git\" command in, needs to be\n> adjusted after \"git\" chdir's up to the top-level of the working\n> tree, and the way to do so is to prepend the \"prefix\" computed by\n> \"git\" (which always ends with a slash). Adding the prefix to\n> relative paths is the central part of what it has to do. When the\n> end-user supplied path goes up e.g., \"../file\", we may have to lose\n> a level or more of the prefix before prepending, but that does not\n> change the fact that the helper function is about prepending the\n> prefix.\n> \n> And as you can guess from the above description, if the caller\n> passes prefix that does not end in a trailing slash, the caller is\n> buggy.\n> \n\nSure. Part of the reason I was thinking modify this here instead of\nadding the trailing slash, is because to add a trailing slash I had to\nconvert the path to a strbuf, where as by inserting it here thats\nhandled by the format specifiers.\n\n>> If the prefix does not end in a trailing slash, the\n>> function will combine the last component of the prefix with the first\n>> component of the relative path. This is unlikely to produce a desirable\n>> result.\n> \n> True, and I do not mind being lenient to buggy callers, but given\n> that the majority of callers (i.e. all the existing callers) not\n> being buggy, I wonder if it is better to check and append a slash to\n> the end by a new caller that feeds a prefix that directly comes from\n> the end user?\n> \n> Unlike prefix that is internally generated by Git, we'd need to be a\n> lot more careful if we are taking end-user input directly and\n> passing it as a \"prefix\" (I take that the possible lack of trailing\n> slash as a signal that you are doing something like that in this\n> series). We may need to check and correct that they do not contain\n> \"./\", \"//\", or \"../\", for example, anyway, and adding the required\n> trailing slash at the end sounds like something we may want to do as\n> part of that input validation and massaging _before_ calling these\n> helper functions.\n\nYea, I think it boils down to me passing the two paths from the user in\nas the prefix or root of the pathspecs in parse_pathspec.\n\nPerhaps we either need a different function to clean up the path first\nor modify the pathspec code to not use prefix_path here at all and use\nsomething else instead.\n"},{"id":"518538","messageId":"70e459b2-b80a-4de6-bf7d-47606b60608f@intel.com","threadId":"63483","inReplyTo":"xmqq7c2bl51c.fsf@gitster.g","subject":"Re: [PATCH v3 2/4] pathspec: expose match_pathspec_with_flags","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T22:38:24Z","receivedAt":"2025-05-20T22:39:12Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 5/20/2025 7:39 AM, Junio C Hamano wrote:\n> Jacob Keller writes:\n> \n>> From: Jacob Keller \n>>\n>> The do_match_pathspec() function has the DO_MATCH_LEADING_PATHSPEC\n>> option to allow pathspecs to match when matching \"src\" against a\n>> pathspec like \"src/path/...\". This support is not exposed by\n>> match_pathspec, and the internal flags to do_match_pathspec are not\n>> exposed outside of dir.c\n>>\n>> Make match_pathspec_with_flags public, and expose the\n>> DO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY flags. The\n>> DO_MATCH_EXCLUDE flag is kept private in dir.c\n>>\n>> This will be used in a an extension to support pathspec matching in git\n>> diff --no-index.\n>>\n>> Signed-off-by: Jacob Keller \n>> ---\n>> pathspec.h | 8 ++++++++\n>> dir.c | 11 +++++------\n>> 2 files changed, 13 insertions(+), 6 deletions(-)\n> \n> You use diff.orderfile? Not complaining, just finding it amusing\n> that somebody uses the feature ;-).\n> \n\nOne of my coworkers asked me to set it up so that header files appeared\nfirst in diffs. I kinda liked that, so I stuck with it.\n\n>> diff --git a/pathspec.h b/pathspec.h\n>> index de537cff3cb6..d22d4e80248d 100644\n>> --- a/pathspec.h\n>> +++ b/pathspec.h\n>> @@ -184,6 +184,14 @@ int match_pathspec(struct index_state *istate,\n>> \t\t const char *name, int namelen,\n>> \t\t int prefix, char *seen, int is_dir);\n>> \n>> +#define DO_MATCH_DIRECTORY (1<<1)\n>> +#define DO_MATCH_LEADING_PATHSPEC (1<<2)\n>> +\n>> +int match_pathspec_with_flags(struct index_state *istate,\n>> +\t\t\t const struct pathspec *ps,\n>> +\t\t\t const char *name, int namelen,\n>> +\t\t\t int prefix, char *seen, unsigned flags);\n>> +\n>> /*\n>> * Determine whether a pathspec will match only entire index entries (non-sparse\n>> * files and/or entire sparse directories). If the pathspec has the potential to\n>> diff --git a/dir.c b/dir.c\n>> index a374972b6243..2f2b654b0252 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -329,9 +329,8 @@ static int do_read_blob(const struct object_id *oid, struct oid_stat *oid_stat,\n>> \treturn 1;\n>> }\n>> \n>> +// DO_MATCH_EXCLUDE is not public\n> \n> We do not use // comments (outside borrowed code anyway).\n> \n\nSure. I don't actually expect to keep this patch as-is anyways, since I\nthink we might want to do something else... as exposing these flags\nseems incorrect to me...\n\n>> #define DO_MATCH_EXCLUDE (1<<0)\n>> -#define DO_MATCH_DIRECTORY (1<<1)\n>> -#define DO_MATCH_LEADING_PATHSPEC (1<<2)\n>> \n\nI actually almost wonder if we should set both DO_MATCH_DIRECTORY and\nDO_MATCH_LEADING_PATHSPEC in match_pathspec when is_dir is true.\n\nThe DO_MATCH_DIRECTORY causes pathspecs to match if we have a path like\n\"a/b/c/d\" and a pathspec like \"a/b/c\".\n\nThe DO_MATCH_LEADING_PATHSPEC does the inverse: if we have a path like\n\"a/b/c\" then we match a pathspec like \"a/b/c/d\"\n\nI guess it really depends on the nature of the caller. In the normal\ncase, I guess we don't check intermediate directory paths and only check\nthe end resulting files. But in my case, we're checking \"a/b/c\" before\nwe descend into it to check its inner contents.\n"},{"id":"518539","messageId":"85b74be2-6fb9-4a00-aa8f-f6e25c383e91@intel.com","threadId":"63483","inReplyTo":"xmqqwmabjoww.fsf@gitster.g","subject":"Re: [PATCH v3 3/4] pathspec: add flag to indicate operation without repository","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T22:42:08Z","receivedAt":"2025-05-20T22:42:18Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 5/20/2025 8:13 AM, Junio C Hamano wrote:\n> Jacob Keller writes:\n> \n>> From: Jacob Keller \n>>\n>> A following change will add support for pathspecs to the git diff\n>> --no-index command. This mode of git diff does not load any repository.\n>>\n>> Add a new PATHSPEC_NO_REPOSITORY flag indicating that we're parsing\n>> pathspecs without a repository.\n>>\n>> Both PATHSPEC_ATTR and PATHSPEC_FROMTOP require a repository to\n>> function. Thus, verify that both of these are set in magic_mask to\n>> ensure they won't be accepted when PATHSPEC_NO_REPOSITORY is set.\n>>\n>> Check PATHSPEC_NO_REPOSITORY when warning about paths outside the\n>> directory tree. When the flag is set, do not look for a git repository\n>> when generating the warning message.\n>>\n>> Finally, add a BUG in match_pathspec_item if the istate is NULL but the\n>> pathspec has PATHSPEC_ATTR set. Callers which support PATHSPEC_ATTR\n>> should always pass a valid istate, and callers which don't pass a valid\n>> istate should have set PATHSPEC_ATTR in the magic_mask field to disable\n>> support for attribute-based pathspecs.\n> \n> All very sensible considerations.\n> \n>> diff --git a/dir.c b/dir.c\n>> index 2f2b654b0252..45aac0bfacab 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -396,9 +396,12 @@ static int match_pathspec_item(struct index_state *istate,\n>> \t strncmp(item->match, name - prefix, item->prefix))\n>> \t\treturn 0;\n>> \n>> -\tif (item->attr_match_nr &&\n>> -\t !match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n>> -\t\treturn 0;\n>> +\tif (item->attr_match_nr) {\n>> +\t\tif (!istate)\n>> +\t\t\tBUG(\"magic PATHSPEC_ATTR requires an index\");\n>> +\t\tif (!match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n>> +\t\t\treturn 0;\n>> +\t}\n> \n> It is a bit curious why we do not check PATHSPEC_NO_REPOSITORY here,\n> but it is OK, because it is a BUG for istate to be NULL when we have\n> a repository anyway.\n> \n\nRight. We could check it here, but I actually had added this BUG first\nbefore I added PATHSPEC_NO_REPOSITORY.\n\n>> diff --git a/pathspec.c b/pathspec.c\n>> index 2b4e434bc0aa..a3ddd701c740 100644\n>> --- a/pathspec.c\n>> +++ b/pathspec.c\n>> @@ -492,7 +492,7 @@ static void init_pathspec_item(struct pathspec_item *item, unsigned flags,\n>> \t\tif (!match) {\n>> \t\t\tconst char *hint_path;\n>> \n>> -\t\t\tif (!have_git_dir())\n>> +\t\t\tif ((flags & PATHSPEC_NO_REPOSITORY) || !have_git_dir())\n>> \t\t\t\tdie(_(\"'%s' is outside the directory tree\"),\n>> \t\t\t\t copyfrom);\n>> \t\t\thint_path = repo_get_work_tree(the_repository);\n> \n> This is a part of generating an error message. We die early to\n> avoid having to call get-work-tree when we know we are not even in\n> any working tree, which makes sense.\n> \n>> @@ -614,6 +614,10 @@ void parse_pathspec(struct pathspec *pathspec,\n>> \t (flags & PATHSPEC_PREFER_FULL))\n>> \t\tBUG(\"PATHSPEC_PREFER_CWD and PATHSPEC_PREFER_FULL are incompatible\");\n>> \n>> +\tif ((flags & PATHSPEC_NO_REPOSITORY) &&\n>> +\t (~magic_mask & (PATHSPEC_ATTR | PATHSPEC_FROMTOP)))\n>> +\t\tBUG(\"PATHSPEC_NO_REPOSITORY is incompatible with PATHSPEC_ATTR and PATHSPEC_FROMTOP\");\n> \n> Hmph, I am not sure if this change is correct. The magic_mask\n> parameter is passed by a caller to say \"even if parsr_pathspec()\n> parses a pathspec using a certain set of features properly, the\n> caller is not prepared to handle the parsed result\". If magic_mask\n> lacks PATHSPEC_ATTR, that does not necessarily mean that the given\n> pathspec contains any pathspec items that do use the attr magic. It\n> merely says that the caller is not prepared to handle a pathspec\n> item that uses the attr magic feature.\n> \n\nRight. The magic_mask is a \"these magic types are not allowed\". I'm\nchecking to make sure that if you set PATHSPEC_NO_REPOSITORY, you must\nalso set PATHSPEC_ATTR and PATHSPEC_FROMTOP, because you cannot possibly\nhandle these pathspecs without a repository.\n\n> If we are going to add a call to parse_pathspec() in a code path\n> that is specific to diff-no-index, isn't it sufficient to pass\n> PATHSPEC_ATTR and PATHSPEC_FROMTOP as magic_mask without this\n> change?\n> \n\nStrictly speaking, yes. This part is really just a \"this would be a\nprogrammer error we should catch early\".\n"},{"id":"518540","messageId":"5343efa2-99b2-4532-b52d-0e120678470c@intel.com","threadId":"63483","inReplyTo":"xmqqy0uri6st.fsf@gitster.g","subject":"Re: [PATCH v3 4/4] diff --no-index: support limiting by pathspec","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T22:45:28Z","receivedAt":"2025-05-20T22:45:32Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 5/20/2025 9:30 AM, Junio C Hamano wrote:\n> Jacob Keller writes:\n>> -`git diff [] --no-index [--] `::\n>> +`git diff [] --no-index [--] [...]`::\n> \n> This is a bit unfortunate. The disambiguating \"--\" should ideally\n> be between the \"things to be compared\" and the pathspec, as the\n> former corresponds to in the normal \"git diff\" invocation.\n> \n\nTrue, but it looks like we already had it before paths. I am not sure if\nwe can easily change that now. :(\n\n>> +\t... If both\n>> +\tpaths point to directories, additional pathspecs may be\n>> +\tprovided. These will limit the files included in the\n>> +\tdifference. All such pathspecs must be relative as they\n>> +\tapply to both sides of the diff.\n> \n> \"as they\" -> \"and they\"?\n\nI think I meant \"because they\", but for documentation I think and they\nmakes more sense. I can clarify this a bit better in the next version.\n\n> \n>> +test_expect_success 'diff --no-index with pathspec' '\n>> +\ttest_expect_code 1 git diff --no-index a b 1 >actual &&\n>> +\tcat >expect <<-EOF &&\n>> +\tdiff --git a/a/1 b/a/1\n>> +\tdeleted file mode 100644\n>> +\tindex d00491f..0000000\n>> +\t--- a/a/1\n>> +\t+++ /dev/null\n>> +\t@@ -1 +0,0 @@\n>> +\t-1\n>> +\tEOF\n>> +\ttest_cmp expect actual\n>> +'\n> \n> If you use --name-only or --name-status would the test become\n> simpler?\n> \n\nThat is a good idea.\n\n>> +\n>> +test_expect_success 'diff --no-index with pathspec no matches' '\n>> +\ttest_expect_code 0 git diff --no-index a b missing\n>> +'\n> \n> OK.\n> \n>> +test_expect_success 'diff --no-index with negative pathspec' '\n>> +\ttest_expect_code 1 git diff --no-index a b \":!2\" >actual &&\n>> +\tcat >expect <<-EOF &&\n>> +\tdiff --git a/a/1 b/a/1\n>> +\tdeleted file mode 100644\n>> +\tindex d00491f..0000000\n>> +\t--- a/a/1\n>> +\t+++ /dev/null\n>> +\t@@ -1 +0,0 @@\n>> +\t-1\n>> +\tEOF\n>> +\ttest_cmp expect actual\n>> +'\n> \n> OK.\n> \n> All other tests also look sensible.\n> \n> Thanks.\n\n"},{"id":"518541","messageId":"fd82cfe6-0a3b-4f1c-b9a2-91d61ddc9391@intel.com","threadId":"63483","inReplyTo":"xmqqy0uri6st.fsf@gitster.g","subject":"Re: [PATCH v3 4/4] diff --no-index: support limiting by pathspec","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-20T22:47:36Z","receivedAt":"2025-05-20T22:47:42Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 5/20/2025 9:30 AM, Junio C Hamano wrote:\n> Jacob Keller writes:\n> \n>> The git diff --no-index mode does not support pathspecs, and cannot\n>> limit the diff output in this way. Other diff programs such as GNU\n>> difftools have options for excluding paths based on a pattern match.\n> \n> True.\n> \n>> However, using git diff as a diff replacement has several advantages\n>> over many popular diff tools, including coloring moved lines, rename\n>> detections, and similar.\n> \n> I said this when we introduced \"--no-index\", but back when \"git\" was\n> still young, it would have helped a lot wider developer populations\n> if we didn't do \"git diff --no-index\" and instead donated these\n> features to \"many popular diff tools\". We however chose to be lazy\n> and selfish---those who want to use these features are better off\n> installing \"git\" even if they are not using it for version control,\n> only to use it as \"better diff\".\n> \n> People are still welcome to add \"coloring moved lines, rename\n> detections and similar\" to \"many popular diff tools\", but given that\n> \"git\" has become fairly popular and widely used, it may not matter\n> all that much any more that we were lazy and selfish ;-).\n> \n>> Teach git diff --no-index how to handle pathspecs to limit the\n>> comparisons. This will only be supported if both provided paths are\n>> directories.\n> \n> Good. If you are giving only two files, pathspec limiting would not\n> make much sense. If you are giving a file and a directory, lazily\n> give only F and D to compare F with D/F, that is essentially giving\n> only two files F and D/F, so pathspec limiting would not make much\n> sense, either. pathspec limited comparison would make sense only\n> when you are talking about two sets of files.\n> \n>> However, if we always passed DO_MATCH_LEADING_PATHSPEC, then we will\n>> incorrectly match in certain cases such as matching 'a/c' against\n>> ':(glob)**/d'. The match logic will see that a matches the leading part\n>> of the **/ and accept this even tho c doesn't match. The trick seems to\n>> be setting both DO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY when\n>> checking directories, but set neither of them when checking files.\n> \n> Sounds sensible.\n> \n>> Some other gotchas and open questions:\n>>\n>> 1) pathspecs must all come after the first two path arguments, you\n>> can't re-arrange them to come first. I'm treating them sort of like\n>> the treeish arguments to git diff-tree.\n> \n> Exactly. \"git diff\" proper is about comparing two sets of files,\n> either comparing two tree-ishes \"git diff master next\", comparing a\n> tree-ish and the index \"git diff --cached HEAD\", comparing a\n> tree-ish and the working tree files \"git diff HEAD\", or comparing\n> the index and the working tree files \"git diff\". It is a natural\n> extension that \"git --no-index dirA dirB\" compares contents of the\n> two directories. In all of these forms, it is common that the\n> comparison can be pathspec limited by giving pathspec elements as\n> the command line arguments at the end.\n> \n>> 2) The pathspecs are interpreted relative to the provided paths, and\n>> thus will always need to be specified as relative paths, and will be\n>> interpreted as relative to the root of the search for each path\n>> separately.\n> \n> Yes, that is not anything new or something we need to point out as\n> if it is any different from the \"normal\" pathspec.\n> \n>> 3) negative pathspecs have to be fully qualified from the root, i.e.\n>> ':(exclude)file' will only exclude 'a/file' and not 'a/b/file'\n>> unless you also use '(glob)' or similar. I think this matches the\n>> other pathspec support, but I an not 100% sure.\n> \n> I think that is correct \"git ls-files :(exclude)po\" does not exclude\n> git-gui/po, for example.\n> \nSounds like you answered most of my open questions. I guess my remaining\nbig question to you or others on the list is whether or not the overall\nalgorithm makes sense, especially in regards to how we handle needing\nboth DO_MATCH_DIRECTORY and DO_MATCH_LEADING_PATHSPEC, and in using two\nseparate calls to parse_pathspec, one set for each path.\n"},{"id":"518627","messageId":"aa11a7ae-d7ba-445f-b3fa-a180843682c1@intel.com","threadId":"63483","inReplyTo":"xmqqwmabjoww.fsf@gitster.g","subject":"Re: [PATCH v3 3/4] pathspec: add flag to indicate operation without repository","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2025-05-21T23:05:09Z","receivedAt":"2025-05-21T23:07:18Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"\n\nOn 5/20/2025 8:13 AM, Junio C Hamano wrote:\n> Jacob Keller writes:\n> \n>> From: Jacob Keller \n>>\n>> A following change will add support for pathspecs to the git diff\n>> --no-index command. This mode of git diff does not load any repository.\n>>\n>> Add a new PATHSPEC_NO_REPOSITORY flag indicating that we're parsing\n>> pathspecs without a repository.\n>>\n>> Both PATHSPEC_ATTR and PATHSPEC_FROMTOP require a repository to\n>> function. Thus, verify that both of these are set in magic_mask to\n>> ensure they won't be accepted when PATHSPEC_NO_REPOSITORY is set.\n>>\n>> Check PATHSPEC_NO_REPOSITORY when warning about paths outside the\n>> directory tree. When the flag is set, do not look for a git repository\n>> when generating the warning message.\n>>\n>> Finally, add a BUG in match_pathspec_item if the istate is NULL but the\n>> pathspec has PATHSPEC_ATTR set. Callers which support PATHSPEC_ATTR\n>> should always pass a valid istate, and callers which don't pass a valid\n>> istate should have set PATHSPEC_ATTR in the magic_mask field to disable\n>> support for attribute-based pathspecs.\n> \n> All very sensible considerations.\n> \n>> diff --git a/dir.c b/dir.c\n>> index 2f2b654b0252..45aac0bfacab 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -396,9 +396,12 @@ static int match_pathspec_item(struct index_state *istate,\n>> \t strncmp(item->match, name - prefix, item->prefix))\n>> \t\treturn 0;\n>> \n>> -\tif (item->attr_match_nr &&\n>> -\t !match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n>> -\t\treturn 0;\n>> +\tif (item->attr_match_nr) {\n>> +\t\tif (!istate)\n>> +\t\t\tBUG(\"magic PATHSPEC_ATTR requires an index\");\n>> +\t\tif (!match_pathspec_attrs(istate, name - prefix, namelen + prefix, item))\n>> +\t\t\treturn 0;\n>> +\t}\n> \n> It is a bit curious why we do not check PATHSPEC_NO_REPOSITORY here,\n> but it is OK, because it is a BUG for istate to be NULL when we have\n> a repository anyway.\n> \nmatch_pathspec doesn't take the same flags as parse_pathspec anyways, so\nwe can't check it here :)\n"}]}