Subject: Re: [PATCH v5 16/18] tree-walk.h API: add a get_tree_entry_path() function

From: Junio C Hamano <[email-masked]>
Date: Thu, 01 Apr 2021 13:41:48 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> -		if (get_tree_entry_mode(r, hash2, del_prefix, shifted, &mode))
> +		if (get_tree_entry_path(r, hash2, del_prefix, shifted))
>  			die("cannot find path %s in tree %s",
>  			    del_prefix, oid_to_hex(hash2));

The observation that many "get_tree_entry()" users do not need the
mode bits and the code can be simplified by taking advantage of
that fact is good.

It does not automatically follow that it is a good implementation of
that simplification to introduce a variant that does not take the
mode pointer.  The original function could have just been taught to
accept NULL in the field the caller is not interested in, as in many
other functions do.

Besides, get_tree_entry_mode() vs get_tree_entry_path() does not
make any sense as pair of functions with contrasting names.  If it
were that unlike the former that returns mode, the latter returns
path instead, it would have made sense, but that is not what is
going on.  The former returns object name and mode, the latter only
returns object name without mode.

In any case, at this point in the series, it is highly dubious that
an extra function is an improvement.  Teaching get_tree_entry() (do
not even rename it) to take NULL in *mode pointer would make a lot
more sense.