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

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?

  (2) "merge" is not the single command that user may make this kind
      of mistake the command could help and use the same helper.
      "git branch myfoo foo" may want to suggest "origin/foo", for
      example.  I just typed "git checkout mater", which could have
      been easily corrected to "git checkout master" with a mechanism
      like this.

Of course, once the suggestion mechanism is in place, it can be used to 
replace unfriendly die()s in every command that takes a ref.

An asterisk sticks to the parameter name, not type

Indent with two HT, not HT followed by a run of SPs.

An overlong line can and should be split

 >
 > Do not add trailing blank lines.
 >

Thanks, will fix this.

When you consider the point (2) above, it becomes clear why this
message does not belong to a helper function with a bland and
generic name "help unknown ref".

This API is misdesigned.  A possible alternative that may be better
reusable would be to have a helper that is used to come up with a
list of suggestions and make the caller responsible for emitting the
error message.

Yes, I think a better name is needed, I was trying to follow along the 
lines of help_unknown_cmd.

However, making the caller responsible for printing the suggestions may 
not be the best alternative. Borrowing from the way help_unknown_cmd 
works, in help_unknown_ref we could:
1) check if autocorrect is on, returning the corrected refname to the 
calling function, otherwise
2) print an error message, a list of suggestions, and exit()

I think this makes for a clean and reusable API, and requires changing 
one line of code in every function that currently calls die().