Re: [PATCH] help.autocorrect: do not run a command if the command given is junk

Johannes Sixt [off-list ref] writes:

From: Johannes Sixt <redacted>

If a given command is not found, then help.c tries to guess which one the
user could have meant. If help.autocorrect is 0 or unset, then a list of
suggestions is given as long as the dissimilarity between the given command
and the candidates is not excessively high. But if help.autocorrect was
non-zero (i.e., a delay after which the command is run automatically), the
latter restriction on dissimilarity was not obeyed.

In my case, this happened:

 $ git ..daab02
 WARNING: You called a Git command named '..daab02', which does not exist.
 Continuing under the assumption that you meant 'read-tree'
 in 4.0 seconds automatically...

The similarity limit that this patch introduces is already used a few lines
later where the list of suggested commands is printed.

Signed-off-by: Johannes Sixt <redacted>
---
 help.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Thanks.  Will apply to 'maint'.

But I am curious about and would prefer to see the story behind '6'
someday.

This is not entirely a new problem that you are introducing, as the same
comparison-with-six appears in a later part of the code, but this patch
duplicates the magic number whose two instances need to match, so in that
sense it makes it more necessary than before to document the choice of
magic number somewhere in a comment in the code.

Did 8af84da (git wrapper: DWIM mistyped commands, 2008-08-31) made up just
a random number out of thin-air?  What bad things would happen if we used
'600' (or '1') instead of '6'?  What kind of correlation does the cut-off
value we use here should have with some intrinsic numbers your git
installation has (e.g. perhaps "must be at least 80% of average length of
available commands" or something like that)?

In the meantime, I think squashing the following in would help us keep the
two magic numbers in sync.

diff --git a/help.c b/help.c
index db888cf..fbf80d9 100644
--- a/help.c
+++ b/help.c

@@ -297,6 +297,9 @@ static void add_cmd_list(struct cmdnames *cmds, struct cmdnames *old)
 	old->names = NULL;
 }
 
+/* how did we decide this is a good cutoff??? */
+#define SIMILAR_ENOUGH(x) ((x) < 6)
+
 const char *help_unknown_cmd(const char *cmd)
 {
 	int i, n, best_similarity = 0;

@@ -331,7 +334,7 @@ const char *help_unknown_cmd(const char *cmd)
 	n = 1;
 	while (n < main_cmds.cnt && best_similarity == main_cmds.names[n]->len)
 		++n;
-	if (autocorrect && n == 1 && best_similarity < 6) {
+	if (autocorrect && n == 1 && SIMILAR_ENOUGH(best_similarity)) {
 		const char *assumed = main_cmds.names[0]->name;
 		main_cmds.names[0] = NULL;
 		clean_cmdnames(&main_cmds);

@@ -349,7 +352,7 @@ const char *help_unknown_cmd(const char *cmd)
 
 	fprintf(stderr, "git: '%s' is not a git-command. See 'git --help'.\n", cmd);
 
-	if (best_similarity < 6) {
+	if (SIMILAR_ENOUGH(best_similarity)) {
 		fprintf(stderr, "\nDid you mean %s?\n",
 			n < 2 ? "this": "one of these");