Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Martin Steigerwald]

Dear Z-Shell developers,

I want to make you aware of the following issue with autocorrection I ran 
into:

autocorrection suggested rm for typing mr without typing "y"
https://bugs.debian.org/844710

Axel analysed this below a bit further an indeed, if I press the space bar, I 
get this:

ms@merkaba:~> cd /tmp       
ms@merkaba:/tmp> LANG=C mr test
zsh: correct 'mr' to 'rm' [nyae]?  
rm: cannot remove 'test': No such file or directory
ms@merkaba:/tmp#1>

So two fixes to consider:

1) Don´t confirm on space, as thats to easy to trigger accidentally. :)

2) Don´t autocorrect to dangerous commands like "rm". Could be a bit 
challenging to make a list of commands which are dangerous and can easily 
trigger unwanted actions. "rm" would IMO definately be one of this, while with 
"dd" it would be harder to trigger an unwanted action by accident due to 
syntax requirements.

Axel made me aware that I tell Z-Shell to ignore dangerous commands with 
CORRECT_IGNORE=rm, but I think it would be good to reconsider the standard 
behavior.

Thank you,
Martin

----------  Weitergeleitete Nachricht  ----------

Betreff: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for 
typing mr without typing "y"
Datum: Freitag, 18. November 2016, 13:59:34 CET
Von: Martin Steigerwald <martin.steigerwald@teamix.de>
An: Axel Beckert <abe@debian.org>
Kopie: 844710@bugs.debian.org

Am Freitag, 18. November 2016, 13:00:24 CET schrieben Sie:
> Hi Martin,
> 
> Martin Steigerwald wrote:
> > ms@intraws:~/Backup/Mail/Linux> mr kernel-ml_archive.gz
> > kernel-ml_archive_2014-1b.gz zsh: correct 'mr' to 'rm' [nyae]?
> > rm: das Entfernen von „kernel-ml_archive_2014-1b.gz“ ist nicht möglich:
> > Datei oder Verzeichnis nicht gefunden
> > 
> > I didn´t type yes, as when I type "y", it is shown on command line:
> > 
> > ms@intraws:~/Backup/Mail/Linux#1> LANG=C mr test
> > zsh: correct 'mr' to 'rm' [nyae]? y
> > rm: cannot remove 'test': No such file or directory
> > 
> > And I really didn´t type "y" there, I am pretty sure of that, but I may
> > have hit another key by accident.
> 
> Indeed scary.
> 
> From the output it look to as if "Enter" had been pressed on a
> first glance. But if I press "Enter" (on Sid at least) it shows an "n"
> instead afterwards. (Since I have mr installed, I tested it with "rmm"
> which is only available if nmh or mailutils-mh is installed.)
> 
> After some experimenting I noticed that while pressing Enter is
> equivalent to pressing "n" and also prints an "n", pressing the space
> bar is equivalent to "y" _without_ printing a "y".
> 
> So you very likely hit the space bar accidentially.

Yikes! Space bar to confirm? And correction to "rm".

Actually what I tried to type was "mv". I obviously wanted to move the file. 
But I accidentelly typed "mr". And then, yes, likely the space bar.

I think this deserves a fix in the software :)

Thanks.

-------------------------------------------------------------

-- 
Martin Steigerwald  | Trainer

teamix GmbH
Südwestpark 43
90449 Nürnberg

Tel.:  +49 911 30999 55 | Fax: +49 911 30999 99
mail: martin.steigerwald@teamix.de | web:  http://www.teamix.de | blog: http://blog.teamix.de

Amtsgericht Nürnberg, HRB 18320 | Geschäftsführer: Oliver Kügow, Richard Müller

teamix Support Hotline: +49 911 30999-112
 
 *** Bitte liken Sie uns auf Facebook: facebook.com/teamix ***

Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Daniel Shahaf]

Martin Steigerwald wrote on Fri, Nov 18, 2016 at 14:15:51 +0100:
> So two fixes to consider:
> 
> 1) Don´t confirm on space, as thats to easy to trigger accidentally. :)

The code confirms on both tabs (since commit 7f1ce570) and spaces (since
before CVS).  Does anyone know a reason for doing this?

The patch also downscopes a couple of local variables, with no
functional change.

diff --git a/README b/README
index 855e764..4834e00 100644
--- a/README
+++ b/README
@@ -116,6 +116,11 @@ are output as "export" commands unless the parameter is also local, and
 other parameters not local to the scope are output with the "-g" option.
 Previously, only "typeset" commands were output, never using "-g".
 
+8) At spelling-correction prompt ($SPROMPT), where the choices offered are
+[nyae], previously <Enter> would be accepted to mean [N] and <Space> and
+<Tab> would be accepted to mean [Y].  Now <Space> and <Tab> are invalid
+choices: typing either of them remains at the prompt.
+
 Incompatibilities between 5.0.8 and 5.2
 ---------------------------------------
 
diff --git a/Src/utils.c b/Src/utils.c
index 7bbd588..7f3ddad 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -2944,9 +2944,7 @@ mod_export void
 spckword(char **s, int hist, int cmd, int ask)
 {
     char *t, *correct_ignore;
-    int x;
     char ic = '\0';
-    int ne;
     int preflen = 0;
     int autocd = cmd && isset(AUTOCD) && strcmp(*s, ".") && strcmp(*s, "..");
 
@@ -3015,6 +3013,7 @@ spckword(char **s, int hist, int cmd, int ask)
     } else {
 	guess = *s;
 	if (*guess == Tilde || *guess == String) {
+	    int ne;
 	    ic = *guess;
 	    if (!*++t)
 		return;
@@ -3059,6 +3058,7 @@ spckword(char **s, int hist, int cmd, int ask)
     if (errflag)
 	return;
     if (best && (int)strlen(best) > 1 && strcmp(best, guess)) {
+	int x;
 	if (ic) {
 	    char *u;
 	    if (preflen) {
@@ -3088,14 +3088,14 @@ spckword(char **s, int hist, int cmd, int ask)
 		free(pptbuf);
 		fflush(shout);
 		zbeep();
-		x = getquery("nyae \t", 0);
+		x = getquery("nyae", 0);
 		if (cmd && x == 'n')
 		    pathchecked = path;
 	    } else
 		x = 'n';
 	} else
 	    x = 'y';
-	if (x == 'y' || x == ' ' || x == '\t') {
+	if (x == 'y') {
 	    *s = dupstring(best);
 	    if (hist)
 		hwrep(best);


On a tangent: what do "nyae" mean?  I couldn't find the answer in the
manual.

> 2) Don´t autocorrect to dangerous commands like "rm". Could be a bit 
> challenging to make a list of commands which are dangerous and can easily 
> trigger unwanted actions. "rm" would IMO definately be one of this, while with 
> "dd" it would be harder to trigger an unwanted action by accident due to 
> syntax requirements.
> 
> Axel made me aware that I tell Z-Shell to ignore dangerous commands with 
> CORRECT_IGNORE=rm, but I think it would be good to reconsider the standard 
> behavior.

Well, it _does_ prompt the user asking whether "rm" is correct.  Isn't
that sufficient?

Cheers,

Daniel

Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Martin Steigerwald]

Am Samstag, 19. November 2016, 07:55:42 CET schrieb Daniel Shahaf:
> Martin Steigerwald wrote on Fri, Nov 18, 2016 at 14:15:51 +0100:
> > So two fixes to consider:
> > 
> > 1) Don´t confirm on space, as thats to easy to trigger accidentally. :)
> 
> The code confirms on both tabs (since commit 7f1ce570) and spaces (since
> before CVS).  Does anyone know a reason for doing this?
> 
> The patch also downscopes a couple of local variables, with no
> functional change.
[…]
> On a tangent: what do "nyae" mean?  I couldn't find the answer in the
> manual.

I thought about this as well.

> > 2) Don´t autocorrect to dangerous commands like "rm". Could be a bit
> > challenging to make a list of commands which are dangerous and can easily
> > trigger unwanted actions. "rm" would IMO definately be one of this, while
> > with "dd" it would be harder to trigger an unwanted action by accident
> > due to syntax requirements.
> > 
> > Axel made me aware that I tell Z-Shell to ignore dangerous commands with
> > CORRECT_IGNORE=rm, but I think it would be good to reconsider the standard
> > behavior.
> 
> Well, it _does_ prompt the user asking whether "rm" is correct.  Isn't
> that sufficient?

Well, I meant the last sentence "I think it would be good to reconsider the 
standard behavior" generally to both of the above items. The current 
combination of 1) and 2) invites accepting dangerous autocorrections 
accidentally. It may be enough to change 1) or 2). 

That written, on other occassions Z-Shell protects the user from accidentally 
triggering potentially dangerous operations as well:

Z-Shell:

martin@merkaba:/tmp/test/test2> echo LANG=C rm -rf .*
LANG=C rm -rf .*
martin@merkaba:/tmp/test/test2> LANG=C rm -rf .*

Bash:

martin@merkaba:/tmp/test/test2> bash
martin@merkaba:/tmp/test/test2 -> echo LANG=C rm -rf .*
LANG=C rm -rf . ..
martin@merkaba:/tmp/test/test2 -> LANG=C rm -rf .*
rm: refusing to remove '.' or '..' directory: skipping '.'
rm: refusing to remove '.' or '..' directory: skipping '..'

(Actually I am glad that coreutils rm -rf skips these meanwhile too. And also 
refused to do rm -rf / unless a special option is given.)

Thank you,
-- 
Martin

Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Frank Terbeck]

Hey,

Martin Steigerwald wrote:
>> On a tangent: what do "nyae" mean?  I couldn't find the answer in the
>> manual.
>
> I thought about this as well.

This would be:

(Y)es, (N)o, (A)bort, (E)dit.


Regards, Frank

Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Daniel Shahaf]

Frank Terbeck wrote on Sat, Nov 19, 2016 at 12:46:42 +0100:
> >> On a tangent: what do "nyae" mean?  I couldn't find the answer in the
> >> manual.
> >
> (Y)es, (N)o, (A)bort, (E)dit.

diff --git a/Doc/Zsh/params.yo b/Doc/Zsh/params.yo
index c7d84b9..0db8a1c 100644
--- a/Doc/Zsh/params.yo
+++ b/Doc/Zsh/params.yo
@@ -1458,6 +1458,14 @@ The prompt used for spelling correction.  The sequence
 `tt(%R)' expands to the string which presumably needs spelling
 correction, and `tt(%r)' expands to the proposed correction.
 All other prompt escapes are also allowed.
+
+The actions available at the prompt are tt([nyae]):
+startsitem()
+sitem(tt(n) +LPAR()`no'+RPAR() +LPAR()default+RPAR())(Discard the correction and run the command.)
+sitem(tt(y) +LPAR()`yes'+RPAR())(Make the correction and run the command.)
+sitem(tt(a) +LPAR()`abort'+RPAR())(Discard the entire command line without running it.)
+sitem(tt(e) +LPAR()`edit'+RPAR())(Resume editing the command line.)
+endsitem()
 )
 vindex(STTY)
 item(tt(STTY))(

Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Bart Schaefer]

On Nov 19,  7:55am, Daniel Shahaf wrote:
} Subject: Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrecti
}
} Martin Steigerwald wrote on Fri, Nov 18, 2016 at 14:15:51 +0100:
} > So two fixes to consider:
} > 
} > 1) Don't confirm on space, as thats to easy to trigger accidentally. :)
} 
} The code confirms on both tabs (since commit 7f1ce570) and spaces (since
} before CVS).  Does anyone know a reason for doing this?

If you have a look at the zsh-workers article referenced by 7f1ce570,
you'll see that both space and tab date from time immemorial; the patch
in 7f1ce570 actually REMOVES the interpretation of tab as "yes" from
the NO_RM_STAR_SILENT option, it didn't change CORRECT handling.

Why it's this way probably has more to do with Paul Falstad's typing
habits than anything else.  If you're used to hitting space or tab at
a completion, and most of the time you believe the correction, then
you probably want space and tab to mean "go ahead" anytime the shell
is waiting for something.

Notice the RM_STAR_WAIT option for a similar situation.

While I don't use CORRECT and therefore personally don't much care
what this prompt does, I question whether one person remarking about
this after 25+ years of it being the way it has been, is a sufficient
reason for us to immediately change it.

Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Daniel Shahaf]

Bart Schaefer wrote on Sat, Nov 19, 2016 at 10:00:23 -0800:
> On Nov 19,  7:55am, Daniel Shahaf wrote:
> } Subject: Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrecti
> }
> } Martin Steigerwald wrote on Fri, Nov 18, 2016 at 14:15:51 +0100:
> } > So two fixes to consider:
> } > 
> } > 1) Don't confirm on space, as thats to easy to trigger accidentally. :)
> } 
> } The code confirms on both tabs (since commit 7f1ce570) and spaces (since
> } before CVS).  Does anyone know a reason for doing this?
> 
> If you have a look at the zsh-workers article referenced by 7f1ce570,
> you'll see that both space and tab date from time immemorial; the patch
> in 7f1ce570 actually REMOVES the interpretation of tab as "yes" from
> the NO_RM_STAR_SILENT option, it didn't change CORRECT handling.
> 

I had looked at it, but missummarized it.  Mea culpa.

> Why it's this way probably has more to do with Paul Falstad's typing
> habits than anything else.  If you're used to hitting space or tab at
> a completion, and most of the time you believe the correction, then
> you probably want space and tab to mean "go ahead" anytime the shell
> is waiting for something.
> 
> Notice the RM_STAR_WAIT option for a similar situation.
> 
> While I don't use CORRECT and therefore personally don't much care
> what this prompt does, I question whether one person remarking about
> this after 25+ years of it being the way it has been, is a sufficient
> reason for us to immediately change it.

I didn't write the patch because a guy on the internet suggested it.
I wrote the patch because the semantics of tab and space at that point
are undocumented and I had been convinced by the argument that using
space to accept a correction violates the principle of least surprise.

If we do keep space and tab then we should document them.  

Daniel

Re: Bug#844710: Fwd: Re: [Pkg-zsh-devel] Bug#844710: autocorrection suggested rm for typing mr without typing "y"

[Peter Stephenson]

On Sat, 19 Nov 2016 21:54:05 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> If we do keep space and tab then we should document them.  

I'm inclined to get rid of them.  They've been undocumented
and not even mentioned in the prompt for a quarter century.
If this hasn't been causing mayhem that's just luck.  We
are not running a service for people who like guessing
features (though this may come as a surprise).

pws