Re: [PATCH v1 23/45] check-ignore: convert to use parse_pathspec

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 12, 2013, 23:09 UTC
Message-ID: <CACsJy8Au4KL7msPr62bvSgGdseWFF=cUJSuB2JzGUXehDD6hng@mail.gmail.com>
In-Reply-To: <20130412150304.GH26949@pacific.linksys.moosehall>

On Sat, Apr 13, 2013 at 1:03 AM, Adam Spiers <git@adamspiers.org> wrote:

Show 25 quoted lines

>> -static int check_ignore(const char *prefix, const char **pathspec)
>> +static int check_ignore(int argc, const char **argv, const char *prefix)
>>  {
>>       struct dir_struct dir;
>> -     const char *path, *full_path;
>>       char *seen;
>>       int num_ignored = 0, dtype = DT_UNKNOWN, i;
>>       struct path_exclude_check check;
>>       struct exclude *exclude;
>> +     struct pathspec pathspec;
>>
>>       /* read_cache() is only necessary so we can watch out for submodules. */
>>       if (read_cache() < 0)
>> @@ -70,31 +70,39 @@ static int check_ignore(const char *prefix, const char **pathspec)
>>       dir.flags |= DIR_COLLECT_IGNORED;
>>       setup_standard_excludes(&dir);
>>
>> -     if (!pathspec || !*pathspec) {
>> +     if (!argc) {
>
> Is there a compelling reason for introducing argc as a new parameter
> to check_ignore(), other than simplifying the above line?  And why
> rename the pathspec parameter to argv?  Both these changes are
> misleading AFAICS, since paths provided to check_ignore() can come
> from sources other than CLI arguments (i.e. via --stdin).

Because I introduced "struct pathspec pathspec;" I need to rename the argument "pathspec" to something else. Maybe we could rename the argument to "paths"?

> The introduction of argc also makes it possible to invoke
> check_ignore() with arguments which are not self-consistent.

This is the same problem with main() and other places that follow this convention. But I don't mind dropping argc either.

-- 
Duy