Unexpected side effect of 'setopt correct'

Unexpected side effect of 'setopt correct'

[Matt Wozniski]

I just noticed this effect of 'setopt correct'.. Is this intentional behavior?

~> zsh -f
mastermind% setopt correct
mastermind% pid() {
zsh: correct 'pid' to 'pic' [nyae]? %


mastermind% function pid() {
function>
mastermind%

Is it intentional that it attempts to correct on a function
definition?  Is it intentional that it only attempts to correct one of
the two types of function definition?  Something about this behavior
caught me off guard.  :)

~Matt

Re: Unexpected side effect of 'setopt correct'

[Peter Stephenson]

On Sun, 5 Aug 2007 22:36:38 -0400
"Matt Wozniski" <godlygeek@gmail.com> wrote:
> I just noticed this effect of 'setopt correct'.. Is this intentional behavior?

Ish.

> ~> zsh -f
> mastermind% setopt correct
> mastermind% pid() {
> zsh: correct 'pid' to 'pic' [nyae]? %
> 
> 
> mastermind% function pid() {
                          ^^ (actually you don't want the () in this case)
> function>
> mastermind%
> 
> Is it intentional that it attempts to correct on a function
> definition?  Is it intentional that it only attempts to correct one of
> the two types of function definition?  Something about this behavior
> caught me off guard.  :)

Correction happens as the line is parsed.  In the first case, the shell
has read as far as "pid", and assumes to begin with it's a command word,
hence it applies correction.  It needs to do this early on because it
has to correct the word before it starts parsing the arguments to it.
To change this would require a lot more lookahead, or we'd have to live
with side effects like not being able to correct to special shell
constructs.

In the second case, since we've found the keyword "function", we know
the next word isn't expected to be an existing command and we can
disable spelling correction immediately,

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: Unexpected side effect of 'setopt correct'

[Matt Wozniski]

On 8/13/07, Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:
> On Sun, 5 Aug 2007 22:36:38 -0400
> "Matt Wozniski" <godlygeek@gmail.com> wrote:
> > I just noticed this effect of 'setopt correct'.. Is this intentional behavior?
>
> Ish.
>
> > mastermind% function pid() {
>                           ^^ (actually you don't want the () in this case)

I've seen you mention before that the () shouldn't be used after
'function', but the grammar says it's allowed:
function word ... [ () ] [ term ] { list }
Either you've made a tiny mistake in the grammar or on the lists.  :)

> > function>
> > mastermind%
> >
> > Is it intentional that it attempts to correct on a function
> > definition?  Is it intentional that it only attempts to correct one of
> > the two types of function definition?  Something about this behavior
> > caught me off guard.  :)
>
> Correction happens as the line is parsed.  In the first case, the shell
> has read as far as "pid", and assumes to begin with it's a command word,
> hence it applies correction.  It needs to do this early on because it
> has to correct the word before it starts parsing the arguments to it.
> To change this would require a lot more lookahead, or we'd have to live
> with side effects like not being able to correct to special shell
> constructs.
>
> In the second case, since we've found the keyword "function", we know
> the next word isn't expected to be an existing command and we can
> disable spelling correction immediately,

I understand what you're saying, but have a tiny suggestion to make: I
understand the necessity of correcting the command before parsing its
arguments, but in the case of "pid() {\n", the first word is not "pid"
but "pid()" - so, I haven't sourcedived on this, but I assume while
parsing out the command, it's iterating over the line, peeking at the
next character, and checking if it's a whitespace or '('...  Wouldn't
it be easy to special-case "word()" to turn off correction in the same
way as "function word" turns it off?  Of course, that wouldn't work as
easily for "word ()" with some whitespace stuck in there, but I'd
consider it a slight improvement if it's as easy a tweak as I think it
would be.  :)

~Matt

Re: Unexpected side effect of 'setopt correct'

[Peter Stephenson]

"Matt Wozniski" wrote:
> > > mastermind% function pid() {
> >                           ^^ (actually you don't want the () in this case)
> 
> I've seen you mention before that the () shouldn't be used after
> 'function', but the grammar says it's allowed:
> function word ... [ () ] [ term ] { list }
> Either you've made a tiny mistake in the grammar or on the lists.  :)

The documentation's right:  the parser will ignore the () if it's
already found the function keyword.

> I understand what you're saying, but have a tiny suggestion to make: I
> understand the necessity of correcting the command before parsing its
> arguments, but in the case of "pid() {\n", the first word is not "pid"
> but "pid()" - so, I haven't sourcedived on this, but I assume while
> parsing out the command, it's iterating over the line, peeking at the
> next character, and checking if it's a whitespace or '('...  Wouldn't
> it be easy to special-case "word()" to turn off correction in the same
> way as "function word" turns it off?  Of course, that wouldn't work as
> easily for "word ()" with some whitespace stuck in there, but I'd
> consider it a slight improvement if it's as easy a tweak as I think it
> would be.  :)

Yes, it looks like it can be done with a bit of hackery in this case.

Index: Src/lex.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/lex.c,v
retrieving revision 1.39
diff -u -r1.39 lex.c
--- Src/lex.c	3 Jun 2007 17:44:20 -0000	1.39
+++ Src/lex.c	13 Aug 2007 21:20:59 -0000
@@ -66,7 +66,12 @@
 /**/
 int inalmore;
 
-/* don't do spelling correction */
+/*
+ * Don't do spelling correction.
+ * Bit 1 is only valid for the current word.  It's
+ * set when we detect a lookahead that stops the word from
+ * needing correction.
+ */
  
 /**/
 int nocorrect;
@@ -344,6 +349,7 @@
     do
 	tok = gettok();
     while (tok != ENDINPUT && exalias());
+    nocorrect &= 1;
     if (tok == NEWLIN || tok == ENDINPUT) {
 	while (hdocs) {
 	    struct heredocs *next = hdocs->next;
@@ -629,7 +635,7 @@
 }
 
 /**/
-int
+static int
 gettok(void)
 {
     int c, d;
@@ -1036,8 +1042,16 @@
 		     *   pws 1999/6/14
 		     */
 		    if (e == ')' || (isset(SHGLOB) && inblank(e) && !bct &&
-				     !brct && !intpos && incmdpos))
+				     !brct && !intpos && incmdpos)) {
+			/*
+			 * Either a () token, or a command word with
+			 * something suspiciously like a ksh function
+			 * definition.
+			 * The current word isn't spellcheckable.
+			 */
+			nocorrect |= 2;
 			goto brk;
+		    }
 		}
 		/*
 		 * This also handles the [k]sh `foo( )' function definition.