From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Tue, 28 Sep 2010 01:08:09 GMT
Subject: Re: [PATCH 3/6] pathspec: cache string length when initialize pathspec
Message-ID: <AANLkTinu5msupME4efxLLRcDPvM3gFd2q+k3RLw6tYuV@mail.gmail.com>
In-Reply-To: <7v62xqx20o.fsf@alter.siamese.dyndns.org>

2010/9/28 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Subject: Re: [PATCH 3/6] pathspec: cache string length when initialize pathspec
>
> s/initialize/initializing/;
>
>> This field will be used when tree_entry_interesting() is converted to
>> use struct pathspec. Currently it uses pathlens[] in struct
>> diff_options to avoid calculating string over and over again.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  cache.h |    3 +++
>>  dir.c   |   11 ++++++++++-
>>  2 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/cache.h b/cache.h
>> index 6227ddb..045c9fc 100644
>> --- a/cache.h
>> +++ b/cache.h
>> @@ -495,6 +495,9 @@ extern int ie_modified(const struct index_state *, struct cache_entry *, struct
>>  struct pathspec {
>>       const char **raw;
>>       int nr;
>> +     struct pathspec_item {
>> +             int len;
>> +     } *items;
>
> Hmm... I would have expected to see
>
>        struct pathspec_item {
>                const char *pattern;
>                int len;
>        };
>        struct pathspec {
>                struct pathspec_item *items;
>                int nr;
>        };
>
> as you would be allocating a structure anyway, but persumably many places
> take their input as a NULL terminated "char **" array, and keeping such an
> array around as the "raw" field may be easier in refactoring.

pathspec_item.pattern is added later on when I implement negative
pathspec so it's not too different from your expectation. "raw"
however is kept. The reason (again in my WIP negative pathspec) is
that I use "!" alone to denote a negative pathspec. So if user gives {
"foo", "!", "foo/bar", NULL }, "raw" will contain exactly that, but
there are only two pathspec_item for "foo" and "foo/bar" (with
negative flag set).
-- 
Duy