Re: [PATCH 0/5] Make :(attr) pathspec work with "git log"
- Date: Mon, 19 Nov 2018 16:31:17 +0100
- From: Duy Nguyen <pclouds@xxxxxxxxx>
- Subject: Re: [PATCH 0/5] Make :(attr) pathspec work with "git log"
On Mon, Nov 19, 2018 at 12:42 PM Ævar Arnfjörð Bjarmason
> On Sun, Nov 18 2018, Nguyễn Thái Ngọc Duy wrote:
> > When :(attr) was added, it supported one of the two main pathspec
> > matching functions, the one that works on a list of paths. The other
> > one works on a tree, tree_entry_interesting(), which gets :(attr)
> > support in this series.
> > With this, "git grep <pattern> <tree> -- :(attr)" or "git log :(attr)"
> > will not abort with BUG() anymore.
> > 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.
> > The main patch is the last one. The others are just to open a path to
> > pass "struct index_state *" down to tree_entry_interesting(). This may
> > become standard procedure because we don't want to stick the_index (or
> > the_repository) here and there.
> Another side-note (this thread is turning into my personal blog at this
> point...) I found an old related thread:
> So this series fixes 1/2 of the issues noted there, but git-ls-tree will
> still die with the same error.
If you mean BUG(), not it does not. There are just a couple more
places besides tree_entry_interesting() and match_pathspec() that need
to be aware of all the magic things. ls-tree is one of those but it's
well guarded and will display this instead
$ git ls-tree HEAD ':(attr:abc=def)'
fatal: :(attr:abc=def): pathspec magic not supported by this command: 'attr'
If you mean making ls-tree support :(attr) and other stuff, it's not
going to happen in near future. It may be nice to switch to
tree_entry_interesting() in this command, but if it was easy to do I
would have done it years ago :P