Re: [PATCH] diffcore: add a filter to find a specific blob
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Dec 27, 2017, 18:59 UTC
- Message-ID
- <20171227185907.GE149622@aiede.mtv.corp.google.com>
- In-Reply-To
- <CAGZ79kYiL0St-600sjxN5gjLSfyRodS1=AqxyALsidntXkg_xA@mail.gmail.com>
Stefan Beller wrote:
> On Thu, Dec 14, 2017 at 6:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 8 quoted lines
>> I think it would make it a better companion to --pickaxe but we need >> to align its behaviour a little bit so that it plays better with the >> "--pickaxe-all" option, and also needs to hide mode and name only >> changes just like pickaxe. > > I looked into this, and the small changes needed led me to thinking > it could be integrated into the diffcore-pickaxe code completely, > roughly like (spaces mangled):
Nice, this looks promising.
[...]
Show 6 quoted lines
> But then, it seems as if any pickaxe option is incompatible with > any other, i.e. from reading the code, you cannot combine -S > and -G, or even give one of them twice. > > I guess that would be not a big deal for the --pickaxe-object, > but just want to point it out.
Agreed that that's not a big deal for --pickaxe-object.
Show 13 quoted lines
>> After all, the diffcore-blobfind code was written while looking at >> the diffcore-pickaxe's code in another window shown in the pager, >> and I tend to agree with your earlier message that this is an >> extreme case of -S<contents> where the contents happens to be the >> whole file. > > I disagree, as the user doesn't have the content, but the hash > over the content only and wants to know more about it. The new > option cannot be used to find a file whose partial content hashes to > the given sha1, either. > > So with these considerations, I would keep the patch as currently\ > queued at sb/diff-blobfind.
Interesting --- I come to the opposite conclusion.
The pickaxe-style behavior seems more consistent and simpler to explain and better matches the use cases I can think of.
Thanks, Jonathan