Junio C Hamano <gitster@xxxxxxxxx> writes:
> Glen Choo <chooglen@xxxxxxxxxx> writes:
>
>> - tree = parse_tree_indirect(old_branch_info->commit ?
>> - &old_branch_info->commit->object.oid :
>> - the_hash_algo->empty_tree);
>> +
>> + old_commit_oid = old_branch_info->commit ?
>> + &old_branch_info->commit->object.oid :
>> + the_hash_algo->empty_tree;
>
> I guess this is done only so that you can use the object name in the
> error message, which is fine.
That's correct.
>> + tree = parse_tree_indirect(old_commit_oid);
>> + if (!tree)
>> + die(_("unable to parse commit %s"),
>> + oid_to_hex(old_commit_oid));
>
> "unable to parse commit" is a bit of a white lie. In fact, there is
> nothing that makes oid_commit_oid the name of a commit object.
>
> "unable to parse object '%s' as a tree" would be more technically
> correct,
Hm, yes. With regards to parse_tree_indirect(), "unable to parse object
'%s' as a tree" is a more accurate description of the failure. But since
we know that the oid is a commit in this context, I'm not sure if we
need to offload that much information to the user - if we failed to
parse the given object id in the appropriate manner, the user would
still be directed to figure out what's wrong with the object.
> but one random-looking hexadecimal string is almost
> indistinguishable from another, and neither would be a very useful
> message from the end user's point of view. I am wondering if we can
> use old_branch_info to formulate something easier to understand for
> the user. update_refs_for_switch() seems to compute old_desc as a
> human readable name by using old_branch_info->name if available
> before falling back to old_branch_info->commit object name, which
> might be a source of inspiration.
I think it's actually more helpful to have the oid instead of a
human-readable description like old_branch_info->name.
For an advanced user/repo admin, seeing the oid makes it very obvious
that there's a particular problem with the given object, and this would
direct them to hunt down the object locally (without partial clone) or
on the remote (with partial clone, as in the original motivation). From
there, it's easy to figure out which branch points to the offending
object. The branch name might be misleading - the user would presumably
start with hunting down the ref, then explore several possibilities
before realizing the problem is actually with the _object_.
For a novice user, neither the branch name nor the object id is
actionable because they probably wouldn't be able to fix the issue
anyway. The advantage of the opaque hex string is that by being
intimidating and unrecognizable, it indicates to the user that they
shouldn't try to debug the issue and so they might give up sooner and
ask for help from someone who might be able to fix it.
