On Sat, Oct 28, 2017 at 10:32 AM, Johannes Schindelin
<[email protected]> wrote:
> Hi Stefan,
>
> On Fri, 27 Oct 2017, Stefan Beller wrote:
>
>> Sometimes users are given a hash of an object and they want to identify
>> it further (ex.: Use verify-pack to find the largest blobs, but what are
>> these? or [1])
>>
>> The best identification of a blob hash is done via a its path at a
>> given commit, which this implements.
>>
>> [1] https://stackoverflow.com/questions/223678/which-commit-has-this-blob
>
> I also came up with a script to do that:
> https://github.com/msysgit/msysgit/blob/master/bin/what-made-this-repo-so-large.sh
>
> Your method is much more elegant, of course (it describes the commit in the
> same run as it finds the object, and it does not output tons of stuff only
> to be filtered).
That was the task I was given. Discussion here turned out that users
mostly only ever care about commit object names, trees and blobs are
only useful when specifically given.
>
>> @@ -282,6 +283,50 @@ static void show_suffix(int depth, const struct
>> object_id *oid)
>> printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
>> }
>>
>> +struct blob_descriptor {
>> + struct object_id current_commit;
>> + struct object_id looking_for;
>> +};
>
> Personally, I would call this process_commit_data, but I do not mind too
> much about the name.
I'll take any naming suggestion, as this code was assembled in a hurry using
copy/paste and trial&error as the high-tech methods used.
>> +static void process_object(struct object *obj, const char *name, void *data)
>> +{
>> + struct blob_descriptor *bd = data;
>> +
>> + if (!oidcmp(&bd->looking_for, &obj->oid))
>> + printf(_("blob %s present at path %s in commit %s\n"),
>> + oid_to_hex(&bd->looking_for), name,
>> + oid_to_hex(&bd->current_commit));
>> +}
>
> s/name/path/
>
>> @@ -295,9 +340,14 @@ static void describe(const char *arg, int last_one)
>>
>> if (get_oid(arg, &oid))
>> die(_("Not a valid object name %s"), arg);
>> - cmit = lookup_commit_reference(&oid);
>> - if (!cmit)
>> - die(_("%s is not a valid '%s' object"), arg, commit_type);
>> + cmit = lookup_commit_reference_gently(&oid, 1);
>> + if (!cmit) {
>> + if (lookup_blob(&oid))
>> + describe_blob(oid);
>> + else
>> + die(_("%s is not a commit nor blob"), arg);
>
> s/not/neither/
>
> Nicely done, sir!
>
> I wonder whether it would make sense to extend this to tree objects while
> we are at it, but maybe that's an easy up-for-grabs.
I can look into incorporating that, too. What is the use case though?
(Is there any error message, common enough that users want to
identify trees?)
Thanks for the review,
Stefan
