Re: [PATCH v3 2/4] pathspec: expose match_pathspec_with_flags

From: Jacob Keller <jacob.e.keller@intel.com>
Date: May 20, 2025, 22:38 UTC
Message-ID: <70e459b2-b80a-4de6-bf7d-47606b60608f@intel.com>
In-Reply-To: <xmqq7c2bl51c.fsf@gitster.g>

On 5/20/2025 7:39 AM, Junio C Hamano wrote:

Show 26 quoted lines

> Jacob Keller <jacob.e.keller@intel.com> writes:
> 
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> The do_match_pathspec() function has the DO_MATCH_LEADING_PATHSPEC
>> option to allow pathspecs to match when matching "src" against a
>> pathspec like "src/path/...". This support is not exposed by
>> match_pathspec, and the internal flags to do_match_pathspec are not
>> exposed outside of dir.c
>>
>> Make match_pathspec_with_flags public, and expose the
>> DO_MATCH_LEADING_PATHSPEC and DO_MATCH_DIRECTORY flags. The
>> DO_MATCH_EXCLUDE flag is kept private in dir.c
>>
>> This will be used in a an extension to support pathspec matching in git
>> diff --no-index.
>>
>> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
>> ---
>>  pathspec.h |  8 ++++++++
>>  dir.c      | 11 +++++------
>>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> You use diff.orderfile?  Not complaining, just finding it amusing
> that somebody uses the feature ;-).
>

One of my coworkers asked me to set it up so that header files appeared first in diffs. I kinda liked that, so I stuck with it.

Show 31 quoted lines

>> diff --git a/pathspec.h b/pathspec.h
>> index de537cff3cb6..d22d4e80248d 100644
>> --- a/pathspec.h
>> +++ b/pathspec.h
>> @@ -184,6 +184,14 @@ int match_pathspec(struct index_state *istate,
>>  		   const char *name, int namelen,
>>  		   int prefix, char *seen, int is_dir);
>>  
>> +#define DO_MATCH_DIRECTORY (1<<1)
>> +#define DO_MATCH_LEADING_PATHSPEC (1<<2)
>> +
>> +int match_pathspec_with_flags(struct index_state *istate,
>> +			      const struct pathspec *ps,
>> +			      const char *name, int namelen,
>> +			      int prefix, char *seen, unsigned flags);
>> +
>>  /*
>>   * Determine whether a pathspec will match only entire index entries (non-sparse
>>   * files and/or entire sparse directories). If the pathspec has the potential to
>> diff --git a/dir.c b/dir.c
>> index a374972b6243..2f2b654b0252 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -329,9 +329,8 @@ static int do_read_blob(const struct object_id *oid, struct oid_stat *oid_stat,
>>  	return 1;
>>  }
>>  
>> +// DO_MATCH_EXCLUDE is not public
> 
> We do not use // comments (outside borrowed code anyway).
>

Sure. I don't actually expect to keep this patch as-is anyways, since I think we might want to do something else... as exposing these flags seems incorrect to me...

>>  #define DO_MATCH_EXCLUDE   (1<<0)
>> -#define DO_MATCH_DIRECTORY (1<<1)
>> -#define DO_MATCH_LEADING_PATHSPEC (1<<2)
>>

I actually almost wonder if we should set both DO_MATCH_DIRECTORY and DO_MATCH_LEADING_PATHSPEC in match_pathspec when is_dir is true.

The DO_MATCH_DIRECTORY causes pathspecs to match if we have a path like "a/b/c/d" and a pathspec like "a/b/c".

The DO_MATCH_LEADING_PATHSPEC does the inverse: if we have a path like "a/b/c" then we match a pathspec like "a/b/c/d"

I guess it really depends on the nature of the caller. In the normal case, I guess we don't check intermediate directory paths and only check the end resulting files. But in my case, we're checking "a/b/c" before we descend into it to check its inner contents.