Re: [PATCH 1/2] help: add help_unknown_ref

From: Junio C Hamano <gitster@pobox.com>
Date: May 1, 2013, 23:07 UTC
Message-ID: <7vzjwe2tai.fsf@alter.siamese.dyndns.org>
In-Reply-To: <5181967E.9020601@gmail.com>

Vikrant Varma <vikrant.varma94@gmail.com> writes:

Show 15 quoted lines

> On 02-05-2013 00:05, Junio C Hamano wrote:
>> If you step back a bit, you would notice two things.
>>
>>   (1) Saying 'foo' when the user means 'origin/foo' is hardly the
>>       only (or even the most common) kind of mistake that the code
>>       you need to add to 'git merge' would encounter and could help
>>       the user with.
>
> Yes. I like your suggestion of using levenshtein.c, similar to what's
> been done in help.c:help_unknown_cmd. However, where do you draw the
> line? Do you also suggest 'remotes/origin/foo' for 'remotes/foo'?
> Also, which would you then prioritize for 'foo': 'fob' (this is local)
> or 'origin/foo'? In other words, what kind of mistakes are you looking
> to correct -  typos, or forgetful omissions, or both and something
> more?

This is your itch, so you get to choose what you would propose to the list and have people tweak it ;-)

I personally think the first version should not use levenshtein _at all_ (hence not solving my 'git checkout mastr'). I however want to make sure the code is structured in such a way that allows other people to build on top later.

> However, making the caller responsible for printing the suggestions
> may not be the best alternative.

That "responsibility" could be encapsulated in a separate helper function the callers call instead of die().

The lower level machinery to come up with a suggested list of refs, when the caller suspects that there is a typo in refs, is the same across different commands, and it is insufficient to have a single helper function that hardcodes "not something we can _MERGE_", and copy the function to create many other helpers that do the same thing but with different error messages.

You would need two layers, one that collects suggested list of refs, and another that is directly called by the top-level caller that takes the refname and the die message, and the latter would use the former. What I was showing was the lower level one, without the higher level one that takes die message and a suspicious user input (the higher level one would be trivial and do not need hand-holding while designing).