Re: [PATCH] diffcore: add a filter to find a specific blob

Hi,

Stefan Beller wrote:

Junio hinted at a different approach of solving this problem, which this
patch implements. Teach the diff machinery another flag for restricting
the information to what is shown. For example:

  $ ./git log --oneline --find-object=v2.0.0:Makefile
  b2feb64309 Revert the whole "ask curl-config" topic for now
  47fbfded53 i18n: only extract comments marked with "TRANSLATORS:"

we observe that the Makefile as shipped with 2.0 was appeared in
v1.9.2-471-g47fbfded53 and in v2.0.0-rc1-5-gb2feb6430b.

Neat!

Nit: s/was appeared/appeared/ (not a big deal though, since this is
already in 'next').

From the above it's not clear to me whether this is about commits
where the object was added or where the object was removed.
Fortunately, the docs are a bit clearer:

                                 ... one side of the diff
       matches the given object id. The object can be a blob,
       gitlink entry or tree (when `-t` is given).

So this appears to be about both additions and removals.  Followup
questions:

- are copies and renames shown (if I am passing -M -C)?
- what about mode changes?  If the file became executable but the
  blob content didn't change, does that commit match?

Nit, not related to this change: it would be nice to have a long
option to go along with the short name '-t' --- e.g. --include-trees.

Another nit: s/gitlink entry/submodule commit/, perhaps.  The commit
object is not a gitlink entry: it is pointed to by a gitlink entry.

Another documentation idea: it may be nice to point out that this
is only about the preimage and postimage submodule commit and that
it doesn't look at the history in between.

                                                         The
reason why these commits both occur prior to v2.0.0 are evil
merges that are not found using this new mechanism.

Would it be worth the doc mentioning that this doesn't look at merges
unless you use one of the -m/-c/--cc options, or does that go without
saying?

[...]

I like this name --find-object more than --blobfind!  I am not sure it
quite matches what the user is looking for, though.  We are not
looking for all occurences of the object; we only care about when the
object appears (was added or removed) in the diff.

Putting it in context, we have:

	pickaxe options:
	-S: detect changes in occurence count of a string
	-G: grep lines in diff for a string

	--pickaxe-all:
		do not filter the diff when the patch matches pickaxe
		conditions.

		kind of like log --full-diff, but restricted to pickaxe
		options.
	--pickaxe-regex: treat -S argument as a regex, not a string

Is this another kind of pickaxe option?  It feels similar to -S, but
at an object level instead of a substring level, so in a way it would
be appealing to call it --pickaxe-object.  Does --pickaxe-all affect
it like it affects -S and -G?

Another context to put it in is:

	--diff-filter:
		limit paths (but not commits?) to those with a change
		matching optarg

If I understand correctly, then --diff-filter does not interact with
--pickaxe-all, or in other words it is a different filtering
condition.  Is this another kind of diff filter?  In that context, it
may be appealing to call it something like --object-filter.

--diff-filter is an example where it seems appealing to have a
--full-diff option to diff-tree that could apply to all filters and
not just pickaxe.

[... implementation snipped ...]

The implementation looks lovely and I'm especially happy about the
tests.  Thanks for writing it.

Thoughts?
Jonathan