vim completion problem after workers/35168

vim completion problem after workers/35168

[İsmail Dönmez]

vim <tab> results in:

_vim:7: parse error near `('

I guess it needs extra quoting but not sure where, so no patch attached :/

ismail

Re: vim completion problem after workers/35168

[Peter Stephenson]

On Mon, 18 May 2015 13:29:03 +0300
İsmail Dönmez <ismail@donmez.ws> wrote:
> vim <tab> results in:
> 
> _vim:7: parse error near `('
> 
> I guess it needs extra quoting but not sure where, so no patch attached :/

No, that's a snafu with getting the right lexical analysis mode for the
pattern.  Was OK for the first one, but got screwed up after "|",
and _vim happens to have a pattern begining with a parenthesis
after a "|".

Thanks for being up to date...

BTW for backward compatibility I'll also need to special case (|...)
which used to match an empty string but is now a parse error.  Should
get around to all that some time this week but it's a busy week...

pws

diff --git a/Src/parse.c b/Src/parse.c
index c938d2d..c486699 100644
--- a/Src/parse.c
+++ b/Src/parse.c
@@ -1174,8 +1174,6 @@ par_case(int *cmplx)
 	if (!strcmp(tokstr, "esac"))
 	    break;
 	str = dupstring(tokstr);
-	incasepat = 0;
-	incmdpos = 1;
 	type = WC_CASE_OR;
 	pp = ecadd(0);
 	palts = ecadd(0);
diff --git a/Test/A01grammar.ztst b/Test/A01grammar.ztst
index 1ba0a54..41fb486 100644
--- a/Test/A01grammar.ztst
+++ b/Test/A01grammar.ztst
@@ -543,6 +543,7 @@
   . ./bad_syntax
 126: Attempt to "." file with bad syntax.
 ?./bad_syntax:2: parse error near `\n'
+# `
 
   echo 'false' >dot_false
   . ./dot_false
@@ -650,3 +651,17 @@
 >Pattern matched five
 >Pattern matched six
 >Character class matched abecedinarian
+
+  case grumph in
+    ( no | (grumph) )
+    print 1 OK
+    ;;
+  esac
+  case snruf in
+    ( fleer | (|snr(|[au]f)) )
+    print 2 OK
+    ;;
+  esac
+0: case patterns within words
+>1 OK
+>2 OK

Re: vim completion problem after workers/35168

[İsmail Dönmez]

Hi,

On Mon, May 18, 2015 at 1:53 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Mon, 18 May 2015 13:29:03 +0300
> İsmail Dönmez <ismail@donmez.ws> wrote:
>> vim <tab> results in:
>>
>> _vim:7: parse error near `('
>>
>> I guess it needs extra quoting but not sure where, so no patch attached :/
>
> No, that's a snafu with getting the right lexical analysis mode for the
> pattern.  Was OK for the first one, but got screwed up after "|",
> and _vim happens to have a pattern begining with a parenthesis
> after a "|".

Unfortunately even after 35184 I get an error:

_vim:7: parse error near `)'

yeah this time its the right parentheses :)

Thanks!

Re: vim completion problem after workers/35168

[Peter Stephenson]

On Mon, 18 May 2015 14:22:20 +0300
İsmail Dönmez <ismail@donmez.ws> wrote:
> Unfortunately even after 35184 I get an error:
> 
> _vim:7: parse error near `)'
> 
> yeah this time its the right parentheses :)

A slightly closer examination reveals the following...

Yet another annoyance: in the old hack, you got away with having a
pattern beginning with "(" (and containing balanced parentheses)
followed by an unbalanced ")".  Now you don't.  I don't think that's
actually *that* unfair, and looking at the context below it was clearly
just a mistake in this case.

pws

diff --git a/Completion/Unix/Command/_vim b/Completion/Unix/Command/_vim
index 021054d..007671b 100644
--- a/Completion/Unix/Command/_vim
+++ b/Completion/Unix/Command/_vim
@@ -4,7 +4,7 @@
 _vim_files () {
   case $PREFIX in
     (+*) _files -P './' $* && return 0 ;;
-    (scp|http(|s)|(|s)ftp):*) _urls ;;
+    ((scp|http(|s)|(|s)ftp):*) _urls ;;
     (*) _files $* ;;
   esac
   case $PREFIX in

Re: vim completion problem after workers/35168

[İsmail Dönmez]

On Mon, May 18, 2015 at 2:40 PM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Mon, 18 May 2015 14:22:20 +0300
> İsmail Dönmez <ismail@donmez.ws> wrote:
>> Unfortunately even after 35184 I get an error:
>>
>> _vim:7: parse error near `)'
>>
>> yeah this time its the right parentheses :)
>
> A slightly closer examination reveals the following...
>
> Yet another annoyance: in the old hack, you got away with having a
> pattern beginning with "(" (and containing balanced parentheses)
> followed by an unbalanced ")".  Now you don't.  I don't think that's
> actually *that* unfair, and looking at the context below it was clearly
> just a mistake in this case.

That fixes the issue. Cheers.

ismail

Re: vim completion problem after workers/35168

[Bart Schaefer]

On May 18, 12:40pm, Peter Stephenson wrote:
} Subject: Re: vim completion problem after workers/35168
}
} On Mon, 18 May 2015 14:22:20 +0300
} Ismail Donmez <ismail@donmez.ws> wrote:
} > _vim:7: parse error near `)'
} 
} Yet another annoyance: in the old hack, you got away with having a
} pattern beginning with "(" (and containing balanced parentheses)
} followed by an unbalanced ")".  Now you don't.

Hrm.  Zsh patterns include "(this|that)", and "case" syntax includes
"pattern)" without an open paren, so ...?

The "old hack" is one of those "well, it really should have been done
differently in the first place, but it wasn't, so now what?" issues
that bug me because *somebody* is going to get bitten in the way _vim
just was.

Re: vim completion problem after workers/35168

[Daniel Shahaf]

Bart Schaefer wrote on Mon, May 18, 2015 at 17:30:34 -0700:
> On May 18, 12:40pm, Peter Stephenson wrote:
> } Subject: Re: vim completion problem after workers/35168
> }
> } On Mon, 18 May 2015 14:22:20 +0300
> } Ismail Donmez <ismail@donmez.ws> wrote:
> } > _vim:7: parse error near `)'
> } 
> } Yet another annoyance: in the old hack, you got away with having a
> } pattern beginning with "(" (and containing balanced parentheses)
> } followed by an unbalanced ")".  Now you don't.
> 
> Hrm.  Zsh patterns include "(this|that)", and "case" syntax includes
> "pattern)" without an open paren, so ...?
> 
> The "old hack" is one of those "well, it really should have been done
> differently in the first place, but it wasn't, so now what?" issues
> that bug me because *somebody* is going to get bitten in the way _vim
> just was.

Is there any case in which the neither pre-35168 code nor post-35168
code gives a parse error, but the semantics are different?

I'm not as worried about previously-valid code becoming syntax error
(I've already found an instance of that "in the wild") as of valid code
becoming valid-but-semantically-different code.

Daniel

Re: vim completion problem after workers/35168

[Mikael Magnusson]

On Tue, May 19, 2015 at 2:30 AM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On May 18, 12:40pm, Peter Stephenson wrote:
> } Subject: Re: vim completion problem after workers/35168
> }
> } On Mon, 18 May 2015 14:22:20 +0300
> } Ismail Donmez <ismail@donmez.ws> wrote:
> } > _vim:7: parse error near `)'
> }
> } Yet another annoyance: in the old hack, you got away with having a
> } pattern beginning with "(" (and containing balanced parentheses)
> } followed by an unbalanced ")".  Now you don't.
>
> Hrm.  Zsh patterns include "(this|that)", and "case" syntax includes
> "pattern)" without an open paren, so ...?
>
> The "old hack" is one of those "well, it really should have been done
> differently in the first place, but it wasn't, so now what?" issues
> that bug me because *somebody* is going to get bitten in the way _vim
> just was.

I agree, there are other places in the distributed functions that are
currently broken by this change, and at least one in my own set of
functions. We can't just break backwards compat like this.

-- 
Mikael Magnusson

Re: vim completion problem after workers/35168

[Peter Stephenson]

Bart Schaefer wrote on Mon, May 18, 2015 at 17:30:34 -0700:
> Hrm.  Zsh patterns include "(this|that)", and "case" syntax includes
> "pattern)" without an open paren, so ...?

The problem is that the initial "(" now has to introduce a set of words
(with whitespace that's optional in most but not all cases) rather than
a single zsh pattern.  If you find an extraneous ")" at the end it's too
late to decide you should have been parsing a single pattern rather than
a set of alternative patterns.  We got away with this before by hacking
the pattern ex post facto in one case but not the other.

We could in principle read it as before and re-parse it, but that's
creating a bug-prone mess to fix a case I doubt happens very often (the
_vim code was clearly intended to have balanced parentheses all along).

> The "old hack" is one of those "well, it really should have been done
> differently in the first place, but it wasn't, so now what?" issues
> that bug me because *somebody* is going to get bitten in the way _vim
> just was.

Yes, we're in rather a corner over this.  But I'm still hoping the nasty
cases can be kept to a minimum.

On Tue, 19 May 2015 01:34:57 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Is there any case in which the neither pre-35168 code nor post-35168
> code gives a parse error, but the semantics are different?

To the best of my knowledge the new code is stricter.  When I get a
moment to look, any laxness I introduce should be exactly on the lines
of what happened before --- I don't think there's any ambiguity because
after a string either you get a "|" (standard) or another string (zsh
special).

pws