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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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().


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]