On Sun, Sep 2, 2012 at 9:50 PM, Adam Spiers <g...@adamspiers.org> wrote:
> 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 :)

Yeah. That was my impression too. I just hoped a new set of eyes might
discover something ;) At lease we could prepare the output format that
can be reused (maybe with little changes) for check-attr debugging if
it comes later. Or make this command part of check-attr..

>> 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.

We could take this opportunity to kill "add --ignore-missing", which
is basically .gitignore checking and it accepts multiple paths, I

>>  - If many paths are given, then perhaps we could print ignored paths
>> (no extra info).
> How is this different to git ls-files -i -o ?

I think ls-files requires real files on working directory, but
check-ignore can deal with just non-existing paths.

>>  - 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.

Mixing include/exclude ignore rules multiple times could be hard to
figure out what goes wrong. But as we haven't seen an actual use case
yet, just leave it out.

>> 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 ;-)

Again I was thinking that check-ignore could work with imaginary paths
and could be used by scripts. If a script just wants to check certain
paths are excluded, it should not need to move to working directory
(though it probably is in working directory most of the time).
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to