On Sun, Nov 18, 2018 at 08:51:52PM +0100, Ævar Arnfjörð Bjarmason wrote:
> > But this also reveals an interesting thing: even though we walk on a
> > tree, we check attributes from _worktree_ (and optionally fall back to
> > the index). This is how attributes are implemented since forever. I
> > think this is not a big deal if we communicate clearly with the user.
> > But otherwise, this series can be scraped, as reading attributes from
> > a specific tree could be a lot of work.
>
> I'm very happy to see this implemented, and I think the behavior
> described here is the right way to go. E.g. in git.git we have diff=perl
> entries in .gitattributes. It would suck if:
>
> git log ':(attr:diff=perl)'
>
> Would only list commits as far as 20460635a8 (".gitattributes: use the
> "perl" differ for Perl", 2018-04-26), since that's when we stop having
> that attribute. Ditto for wanting to run "grep" on e.g. perl files in
> 2.12.0.
>
> I have also run into cases where I want to use a .gitattributes file
> from a specific commit. E.g. when writing pre-receive hooks where I've
> wanted the .gitattributes of the commit being pushed to configure
> something about it. But as you note this isn't supported at all.
>
> But a concern is whether we should be making :(attr:*) behave like this
> for now. Are we going to regret it later? I don't think so, I think
> wanting to use the current working tree's / index's is the most sane
> default, and if we get the ability to read it from revisions as we
> e.g. walk the log it would make most sense to just call that
> :(treeattr:*) or something like that.
I think that ship already sailed with the fact that "git log -p" will
show diffs using the worktree attrs. I agree that it would sometimes be
nice to specify attributes from a particular tree, but at this point the
default probably needs to remain as it is.
-Peff
