threads / patch / 10392

patchOn error, do not list all commands, but point to --help option.

Subject: [PATCH] On error, do not list all commands, but point to --help option.

## tl;dr

16 messages between Oct 20, 2007 and Oct 24, 2007. Diffs are folded; open one to read it.

replies: 15people: 9as markdown or json

Jari Aalto· Oct 20, 2007, 08:24 UTC · lore
- commented out call to list_common_cmds_help()
- Send error message to stderr, not stdout.
Signed-off-by: Jari Aalto <jari.aalto@cante.net>
---
 help.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Show changes to help.c +2 −2
diff --git a/help.c b/help.c
index 1cd33ec..dc9e59f 100644
--- a/help.c
+++ b/help.c
@@ -185,8 +185,8 @@ static void show_man_page(const char *git_cmd)
 
 void help_unknown_cmd(const char *cmd)
 {
-	printf("git: '%s' is not a git-command\n\n", cmd);
-	list_common_cmds_help();
+	fprintf(stderr, "git: '%s' is not a git-command. See --help\n\n", cmd);
+        /* list_common_cmds_help(); */
 	exit(1);
 }
 

-- 
1.5.3.2.81.g17ed

Welcome to FOSS revolution: we fix and modify until it shines
Johannes Schindelin· Oct 20, 2007, 20:28 UTC · re: Jari Aalto · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

Hi,
On Sat, 20 Oct 2007, Jari Aalto wrote:
> - commented out call to list_common_cmds_help()

If you're really sure that this is desired, do not comment it out. Delete it.

But am not at all sure that this is the way to go. Rather, I like it that the most common commands are listed. It would be better to find out what commands are really the most helpful to users who are likely to benefit from the list, and to present them better (such as showing them in categories).

Ciao, Dscho

Jari Aalto· Oct 20, 2007, 22:28 UTC · re: Johannes Schindelin · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

* Sat 2007-10-20 Johannes Schindelin <Johannes.Schindelin@gmx.de> INBOX
Show 8 quoted lines
> Hi,
>
> On Sat, 20 Oct 2007, Jari Aalto wrote:
>
>> - commented out call to list_common_cmds_help()
>
> If you're really sure that this is desired, do not comment it out.  Delete 
> it.
I'm sure.

There is no point of reminding *every* time you make a typo during writing the commands. It fills half of the screen and obscures the previous commands that were running. It's much more polite and elegant in shorted form.

Jari
New patch follows.
-- 
Welcome to FOSS revolution: we fix and modify until it shines
Johannes Schindelin· Oct 20, 2007, 23:02 UTC · re: Jari Aalto · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

Hi,
On Sun, 21 Oct 2007, Jari Aalto wrote:
Show 10 quoted lines
> * Sat 2007-10-20 Johannes Schindelin <Johannes.Schindelin@gmx.de> INBOX
>
> > On Sat, 20 Oct 2007, Jari Aalto wrote:
> >
> >> - commented out call to list_common_cmds_help()
> >
> > If you're really sure that this is desired, do not comment it out.  Delete 
> > it.
> 
> I'm sure.

Well, I'm almost sure of the opposite. One of the big results of the Git Survey was that git is still not user-friendly enough. Your patch would only make this issue worse.

Ciao, Dscho

Shawn O. Pearce· Oct 21, 2007, 02:06 UTC · re: Johannes Schindelin · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 7 quoted lines
> > > On Sat, 20 Oct 2007, Jari Aalto wrote:
> > >
> > >> - commented out call to list_common_cmds_help()
> 
> Well, I'm almost sure of the opposite.  One of the big results of the Git 
> Survey was that git is still not user-friendly enough.  Your patch would 
> only make this issue worse.

Actually I think Jari's patch helps for the reason originally stated in the message (less output when you make a small typo). Though I agree that the commented out code should just be removed.

I actually had to do `git config alias.upsh push` just to keep myself from screaming every time I made a small typo and Git gave me a screenful of "helpful reminders".

Hmm.  Lets see.
  "cvs foo":
    Big spew of commands.  Like "git foo".
  "svn foo":
    Unknown command: 'foo'
    Type 'svn help' for usage.

Both are considered to be more newbie friendly then Git. So clearly SVN's output of almost nothing is friendly. And so is CVS' big spew of frequently used commands. Either way is apparently newbie friendly.

Though I find SVN's message a little insulting, asking me to type. I know I have to type the command, thanks.

-- 
Shawn.
Jeff King· Oct 21, 2007, 03:24 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

On Sat, Oct 20, 2007 at 10:06:53PM -0400, Shawn O. Pearce wrote:
> I actually had to do `git config alias.upsh push` just to keep
> myself from screaming every time I made a small typo and Git gave
> me a screenful of "helpful reminders".
Yeah, somebody should really work on bash completion...
-Peff
Shawn O. Pearce· Oct 21, 2007, 03:29 UTC · re: Jeff King · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

Jeff King <peff@peff.net> wrote:
Show 7 quoted lines
> On Sat, Oct 20, 2007 at 10:06:53PM -0400, Shawn O. Pearce wrote:
> 
> > I actually had to do `git config alias.upsh push` just to keep
> > myself from screaming every time I made a small typo and Git gave
> > me a screenful of "helpful reminders".
> 
> Yeah, somebody should really work on bash completion...
  $ git pu<TAB><TAB>
  pull     push

By the time I type out "pus" and hit tab I've already typed out the name "push ". Except I frequently find myself getting the u before the p, which can't complete. Of course with the above alias in place "git u<TAB>" completes out uniquely to "git push " (between bash completion and the alias expansion).

But that alias isn't there for my bash tab completion. Its there exactly because otherwise "git upsh" gives me 31 lines of useless (to me) output without it.

-- 
Shawn.
Yin Ping· Oct 21, 2007, 06:33 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

On 10/21/07, Shawn O. Pearce <spearce@spearce.org> wrote:
Show 22 quoted lines
> Jeff King <peff@peff.net> wrote:
> > On Sat, Oct 20, 2007 at 10:06:53PM -0400, Shawn O. Pearce wrote:
> >
> > > I actually had to do `git config alias.upsh push` just to keep
> > > myself from screaming every time I made a small typo and Git gave
> > > me a screenful of "helpful reminders".
> >
> > Yeah, somebody should really work on bash completion...
>
>   $ git pu<TAB><TAB>
>   pull     push
>
> By the time I type out "pus" and hit tab I've already typed out
> the name "push ".  Except I frequently find myself getting the
> u before the p, which can't complete.  Of course with the above
> alias in place "git u<TAB>" completes out uniquely to "git push "
> (between bash completion and the alias expansion).
>
> But that alias isn't there for my bash tab completion.  Its there
> exactly because otherwise "git upsh" gives me 31 lines of useless
> (to me) output without it.
>

My way to resolve this is to define some alias begenning with 'gt' for frequently used commands, such as 'gtps -> git-push, gtpl -> git-pull, gtco->git-checkout, gtci->git-commit, gtbr->git-branch'. so that when i type 'gt<TAB>', only commands that i frequently use are listed.

-- 
franky
Wincent Colaiuta· Oct 21, 2007, 08:58 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

El 21/10/2007, a las 4:06, Shawn O. Pearce escribió:
Show 18 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>>> On Sat, 20 Oct 2007, Jari Aalto wrote:
>>>>
>>>>> - commented out call to list_common_cmds_help()
>>
>> Well, I'm almost sure of the opposite.  One of the big results of  
>> the Git
>> Survey was that git is still not user-friendly enough.  Your patch  
>> would
>> only make this issue worse.
>
> Actually I think Jari's patch helps for the reason originally
> stated in the message (less output when you make a small typo).
> Though I agree that the commented out code should just be removed.
>
> I actually had to do `git config alias.upsh push` just to keep
> myself from screaming every time I made a small typo and Git gave
> me a screenful of "helpful reminders".

If you want to go really user friendly, how about a check against the list of known commands using a shortest-edit distance algorithm?

   Unknown command: 'upsh': did you mean 'push'?
   Type 'git help' for usage.

Here's just one of many articles introducing the shortest-edit idea, as popularized by Google:

   <http://norvig.com/spell-correct.html>
What do you think?

Cheers, Wincent

Andreas Ericsson· Oct 21, 2007, 12:42 UTC · re: Wincent Colaiuta · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

Wincent Colaiuta wrote:
Show 23 quoted lines
> El 21/10/2007, a las 4:06, Shawn O. Pearce escribió:
> 
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>>>> On Sat, 20 Oct 2007, Jari Aalto wrote:
>>>>>
>>>>>> - commented out call to list_common_cmds_help()
>>>
>>> Well, I'm almost sure of the opposite.  One of the big results of the 
>>> Git
>>> Survey was that git is still not user-friendly enough.  Your patch would
>>> only make this issue worse.
>>
>> Actually I think Jari's patch helps for the reason originally
>> stated in the message (less output when you make a small typo).
>> Though I agree that the commented out code should just be removed.
>>
>> I actually had to do `git config alias.upsh push` just to keep
>> myself from screaming every time I made a small typo and Git gave
>> me a screenful of "helpful reminders".
> 
> If you want to go really user friendly, how about a check against the 
> list of known commands using a shortest-edit distance algorithm?
> 
http://en.wikipedia.org/wiki/Levenshtein_distance
Implementing the algorithm doesn't seem terribly difficult.
-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Jan Hudec· Oct 21, 2007, 13:13 UTC · re: Andreas Ericsson · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

On Sun, Oct 21, 2007 at 14:42:25 +0200, Andreas Ericsson wrote:
Show 25 quoted lines
> Wincent Colaiuta wrote:
>> El 21/10/2007, a las 4:06, Shawn O. Pearce escribió:
>>> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>>>>> On Sat, 20 Oct 2007, Jari Aalto wrote:
>>>>>>
>>>>>>> - commented out call to list_common_cmds_help()
>>>>
>>>> Well, I'm almost sure of the opposite.  One of the big results of the 
>>>> Git
>>>> Survey was that git is still not user-friendly enough.  Your patch would
>>>> only make this issue worse.
>>>
>>> Actually I think Jari's patch helps for the reason originally
>>> stated in the message (less output when you make a small typo).
>>> Though I agree that the commented out code should just be removed.
>>>
>>> I actually had to do `git config alias.upsh push` just to keep
>>> myself from screaming every time I made a small typo and Git gave
>>> me a screenful of "helpful reminders".
>> If you want to go really user friendly, how about a check against the list 
>> of known commands using a shortest-edit distance algorithm?
>
> http://en.wikipedia.org/wiki/Levenshtein_distance
>
> Implementing the algorithm doesn't seem terribly difficult.

That's not the correct algorithm (you need to consider transpozitions, so you need at least http://en.wikipedia.org/wiki/Damerau-Levenshtein_distance, but I would not think that's the easier, or faster, way to do it. Though this would have to be benchmarked -- calculating the edit distance is quadratic, while generating the list of possibilities (and seeing whether they exist) is linear, but with large constant. So the question is, whether we have few enough commands that the quadratic calculation might be faster.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>
Andreas Ericsson· Oct 21, 2007, 14:17 UTC · re: Jan Hudec · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

Jan Hudec wrote:
Show 32 quoted lines
> On Sun, Oct 21, 2007 at 14:42:25 +0200, Andreas Ericsson wrote:
>> Wincent Colaiuta wrote:
>>> El 21/10/2007, a las 4:06, Shawn O. Pearce escribió:
>>>> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>>>>>>> On Sat, 20 Oct 2007, Jari Aalto wrote:
>>>>>>>
>>>>>>>> - commented out call to list_common_cmds_help()
>>>>> Well, I'm almost sure of the opposite.  One of the big results of the 
>>>>> Git
>>>>> Survey was that git is still not user-friendly enough.  Your patch would
>>>>> only make this issue worse.
>>>> Actually I think Jari's patch helps for the reason originally
>>>> stated in the message (less output when you make a small typo).
>>>> Though I agree that the commented out code should just be removed.
>>>>
>>>> I actually had to do `git config alias.upsh push` just to keep
>>>> myself from screaming every time I made a small typo and Git gave
>>>> me a screenful of "helpful reminders".
>>> If you want to go really user friendly, how about a check against the list 
>>> of known commands using a shortest-edit distance algorithm?
>> http://en.wikipedia.org/wiki/Levenshtein_distance
>>
>> Implementing the algorithm doesn't seem terribly difficult.
> 
> That's not the correct algorithm (you need to consider transpozitions, so you
> need at least http://en.wikipedia.org/wiki/Damerau-Levenshtein_distance, but
> I would not think that's the easier, or faster, way to do it. Though this
> would have to be benchmarked -- calculating the edit distance is quadratic,
> while generating the list of possibilities (and seeing whether they exist) is
> linear, but with large constant. So the question is, whether we have few
> enough commands that the quadratic calculation might be faster.
>

It's intended to be used for strings of length 3-14, run through once when the user has given bogus input. We're not gonna index databases here. Simplicity in the implementation almost certainly outweighs the performance penalty of doing it the stupid way.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
Salikh Zakirov· Oct 24, 2007, 00:28 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] On error, do not list all commands, but point to --help option.

Shawn O. Pearce wrote:
Show 16 quoted lines
> Hmm.  Lets see.
> 
>   "cvs foo":
>     Big spew of commands.  Like "git foo".
> 
>   "svn foo":
>     Unknown command: 'foo'
>     Type 'svn help' for usage.
> 
> Both are considered to be more newbie friendly then Git.  So clearly
> SVN's output of almost nothing is friendly.  And so is CVS'
> big spew of frequently used commands.  Either way is apparently
> newbie friendly.
> 
> Though I find SVN's message a little insulting, asking me to type.
> I know I have to type the command, thanks.

On the contrary, I consider CVS and current Git output more insulting, as it assumes I forgot all the commands every time I made a typo.

The polite part of the message is to point out a typo, as "Unknown command 'foo'" above. The "Type "svn help" for usage" part is just a reminder for complete newbies, which doesn't take too much space and can be reasonably ignored by experienced users.

Jari Aalto· Oct 20, 2007, 22:41 UTC · re: Jari Aalto · lore

[PATCH] On error, do not list all commands, but point to --help option

- Remove out call to list_common_cmds_help()
- Send error message to stderr, not stdout.
Signed-off-by: Jari Aalto <jari.aalto@cante.net>
---
 help.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Show changes to help.c +1 −2
diff --git a/help.c b/help.c
index 1cd33ec..814a8cd 100644
--- a/help.c
+++ b/help.c
@@ -185,8 +185,7 @@ static void show_man_page(const char *git_cmd)
 
 void help_unknown_cmd(const char *cmd)
 {
-	printf("git: '%s' is not a git-command\n\n", cmd);
-	list_common_cmds_help();
+	fprintf(stderr, "git: '%s' is not a git-command. See --help\n\n", cmd);
 	exit(1);
 }
 

-- 
1.5.3.2.81.g17ed
Shawn O. Pearce· Oct 21, 2007, 03:33 UTC · re: Jari Aalto · lore

Re: [PATCH] On error, do not list all commands, but point to --help option

Jari Aalto <jari.aalto@cante.net> wrote:
> - Remove out call to list_common_cmds_help()
Even if the list is against this change (which I'm in favor of)... 
> - Send error message to stderr, not stdout.

I really think this really should be done. CVS and SVN both print to stderr in this case, as does any other program I can think of that takes subcommands. Its just the right thing to do.

Show 7 quoted lines
> @@ -185,8 +185,7 @@ static void show_man_page(const char *git_cmd)
>  
>  void help_unknown_cmd(const char *cmd)
>  {
> -	printf("git: '%s' is not a git-command\n\n", cmd);
> -	list_common_cmds_help();
> +	fprintf(stderr, "git: '%s' is not a git-command. See --help\n\n", cmd);

Why are you still printing two LFs here? We have no additional text to display after this error message, we probably only need the one LF.

-- 
Shawn.
Jan Hudec· Oct 21, 2007, 10:09 UTC · re: Shawn O. Pearce · lore

Re: [PATCH] On error, do not list all commands, but point to --help option

On Sat, Oct 20, 2007 at 23:33:18 -0400, Shawn O. Pearce wrote:
> Jari Aalto <jari.aalto@cante.net> wrote:
> > - Remove out call to list_common_cmds_help()
> 
> Even if the list is against this change (which I'm in favor of)...

I didn't get the impression that the _whole_ list is against. I think that the rather longish list should only be shown for 'git help' and when no command was specified (but that is handled by implying help, so this change does not apply to that case), so I am in favor of this.

-- 
						 Jan 'Bulb' Hudec <bulb@ucw.cz>

← back to recent threads