Re: [PATCH v2 2/2] status: improve rebase todo list parsing

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Junio
>
> On 31/05/2026 01:46, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>> 
>>> +static void abbrev_oid_in_line(struct repository *r,
>>> +			       struct strbuf *line, char **pp)
>>> +{
>>> ...
>>> +	have_oid = !repo_get_oid(r, p, &oid);
>>> +	*end_of_object_name = saved;
>>> +	if (!have_oid)
>>> +		goto out; /* object name was a label */
>> 
>> Can there be a label "deadbeef123" that is unrelated to an object whose
>> object name happens to abbreviate to "deadbeef123"?
>
> In theory yes, but I had assumed it was so unlikely to happen that we 
> could ignore it. If we want to be more careful then we could add a "bool 
> maybe_label" argument for commands that accept a label or a revision and 
> check if "refs/rewritten/$object_name" exists before trying repo_get_oid().

To me, how rare the possibility of such a bug happening is of
secondary importance.  What affects the decision more is when the
"rare" failure happens, if it is immediately obvious to the user,
and if the user may be further harmed badly if they used the wrong
information given by the tool due to such a "rare" failure.

It would be a huge plus if the workaround, when such a "rare"
failure triggers, would be immediately obvious to the user.

What we do not want to see is that the tool to create a wrong
result, cascading into more problems, silently.  In a sense, it is
even worse if such a bug triggers only rarely, because it would mean
that the users always have to be on the lookout.

Having said all that.

I suspect that the OID in the output generated by "status" after it
parses rebase "todo list" is merely meant as an eye candy, and the
users do not _use_ it to decide further actions based on them.

Or do people stare at "git status" output, find an interesting
object name and go "git show" on it or something?  If not, then even
if such a failure were not rare, it would be OK.  We may however
want to record i as a limitation of the current implementation in
the end-user facing documentation, though.

Thanks.