Re: [PATCH 9/9] Add git-check-ignores

> I'm no expert on .gitattributes and check-attr, but AFAICS, all the
> opportunities to share code in the plumbing and front-end seem to be
> taken already, e.g. the directory traversal and path handling.  The
> CLI argument parsing is necessarily different because check-attr
> requires a list of attributes as well as a list of files, and of
> course the output routines have to be different too.
>
> The only opportunity for code reuse which I saw but /didn't/ take was
> around the --stdin line parsing code which is duplicated between:
>
>     check_attr_stdin_paths
>     check_ignore_stdin_paths
>     cmd_checkout_index
>     cmd_update_index
>     hash_stdin_paths
>
> I attempted to refactor these, but quickly realised that due to the
> lack of proper closures in C, the overheads and complexity incurred by
> performing such a refactoring probably outweighed the benefits, so I
> gave up on the idea.
>
> Having said that, I'm totally open to suggestions if you can spot
> other places where code could be reused :)

>> Also --quiet option, where check-ignore returns 0 if the given path is
>> ignored, 1 otherwise?
>
> I considered that, but couldn't think of appropriate behaviour when
> multiple paths are given, so in the end I decided to remain consistent
> with check-attr, which always returns 0.  But I'm happy to change it
> if you can think of a more useful behaviour.  For example we could
> have a --count option which produces no output but has an exit status
> corresponding to the number of ignored files.

>>  - For debugging, given one path, we print all the rules that are
>> applied to it, which may help understand how/why it goes wrong.
>
> That would be nice, but I'm not sure it's a tremendously common use
> case.  Could you think of a scenario in which it would be useful?  I
> guess it could be done by adding a new DIR_DEBUG_IGNORED flag to
> dir_struct which would make the exclude matcher functions collect all
> matching patterns, rather than just returning the first one.  This in
> turn would require another field for collecting all matched patterns.

>> I don't think we really need NEED_WORK_TREE here. .gitignore can be
>> read from index only.
>
> I thought about that, but in the end I decided it probably didn't make
> sense, because none of the exclude matching routines match against the
> index - they all match against the working tree and core.excludesfile.
> This would also require changing the matching logic to honor the index,
> but I didn't see the benefit in doing that, since all operations which
> involve excludes (add, status, etc.) relate to a work tree.
>
> But as with all of the above, please don't hesitate to point out if
> I've missed something.  You guys are the experts, not me ;-)