BUG: Unclosed regexp collection breaks :substitute

[Ingo Karkat]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello Vim developers,

it's possible to avoid escaping a "[" character:

,----[ :help E769 ]----
| When the ']' is not there Vim will not give an error message but
| assume no collection is used. Useful to search for '['.
`----

But when using that feature in a :substitute command, the replacement
part is mistakenly added to the pattern:

:s/[//g
E486: Pattern not found: [//g

I guess the command parser doesn't backtrack when the closing "]"
isn't found, and instead uses the remainder as the pattern (even
though the "/" separators aren't escaped and therefore are valid
separators).

Though this came up in a (wrongly typed) command [1], it's unlikely
that this problem occurs during interactive use. I'm more worried
about plugins automatically inserting the last search pattern into a
generated command and then failing unexpectedly, e.g.:

execute 'substitute/' . @/ . '//g'

- -- regards, ingo

[1] http://vi.stackexchange.com/questions/2538/regex-how-to-replace
- --
-- Ingo Karkat -- /^-- /^-- /^-- /^-- /^-- http://ingo-karkat.de/ --
-- http://vim.sourceforge.net/account/profile.php?user_id=9713 --
Using Vim for 13 years now, mostly 'cos I can't figure out how to exit it.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)

iQEcBAEBAgAGBQJVA056AAoJEA7ziXlAzQ/vhIAH/ij6W9Iz3GnkG0ReeB9c8AVf
2ILB+V/YgIPNvKWR4GHgPgcsaAS7tMFDpYwi0RLuLBVGQAR31rlcqnmPUDYxj8ip
ZjUh0y5nkUUPOuW41YD2Njy6eY0GWAB0PO3NRxJRYChhHPV650vV3M/zav412N6N
Tv9/0hE4ItxTl1V0MW85T60vuUv6M5cFgyHV4H0GJ6FHPdCjt5x/vwKej1yki4fa
LMLti7p0auinbcHItWimjHdRnwKToaR2g1hny9D19k6N0jpDapRp6rA3mMLTMUWW
4+GX6+9ZYi1WEHzqdTAJwWX1lbJhm1DR2vA/NLT3/xm0opIkH3i5FEyYesQqBf8=
=mxIh
-----END PGP SIGNATURE-----

[Christian Brabandt]

Hi Ingo!

On Fr, 13 Mär 2015, Ingo Karkat wrote:

> it's possible to avoid escaping a "[" character:
>
> ,----[ :help E769 ]----
> | When the ']' is not there Vim will not give an error message but
> | assume no collection is used. Useful to search for '['.
> `----
>
> But when using that feature in a :substitute command, the replacement
> part is mistakenly added to the pattern:
>
> :s/[//g
> E486: Pattern not found: [//g
>
> I guess the command parser doesn't backtrack when the closing "]"
> isn't found, and instead uses the remainder as the pattern (even
> though the "/" separators aren't escaped and therefore are valid
> separators).
>
> Though this came up in a (wrongly typed) command [1], it's unlikely
> that this problem occurs during interactive use. I'm more worried
> about plugins automatically inserting the last search pattern into a
> generated command and then failing unexpectedly, e.g.:
>
> execute 'substitute/' . @/ . '//g'

This has happened to me in the past several times. So I would strongly
suggest the docs to discourage the use of it

Best,
Christian
--
Altranft, der
Geruch leerstehender Wohnungen und Wochenendhäuser.
-- Douglas Adams, John Lloyd, Sven Böttcher ("Der tiefere Sinn des Labenz")

[James McCoy]

On Fri, Mar 13, 2015 at 09:54:18PM +0100, Ingo Karkat wrote:
> it's possible to avoid escaping a "[" character:
>
> ,----[ :help E769 ]----
> | When the ']' is not there Vim will not give an error message but
> | assume no collection is used. Useful to search for '['.
> `----
>
> But when using that feature in a :substitute command, the replacement
> part is mistakenly added to the pattern:
>
> :s/[//g
> E486: Pattern not found: [//g

No, that's not what's happening. You can leave off the entire
replacement and the delimiter before it. When this happens, Vim treats
it as deleting the matching strings. To quote:

If the {string} is omitted the substitute is done as if it's empty. Thus the
matched pattern is deleted. The separator after {pattern} can also be left
out then. Example: >
:%s/TESTING
This deletes "TESTING" from all lines, but only one per line.

Cheers,
--
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jame...@jamessan.com>

[Ingo Karkat]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Right. My point is that because the "/" delimiters are not actually
left off (they are there, in the correct, unescaped form), the :s
command *mistakenly* runs into the case you've quoted. Putting it yet
another way, the "[" consumes the following characters (including the
unescaped separators), assuming they belong to the collection, and
when at the end the collection isn't closed, the parsing should
backtrack and reinterpret, but it currently doesn't.

- -- regards, ingo

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)

iQEcBAEBAgAGBQJVA/McAAoJEA7ziXlAzQ/vzY8H/iDaGOuDaf37RwtzzjInRVWO
U3pWmVO5JGkJb+CZTBNRPBEZmL+5NewPooyAf0vmh+AimXJFl53AQbwoX0sdtr2S
x2BqT39WlsTh9DGlDA3TNGGy2buDGEwkjDSDhFckgTWB+6O52H4WcKtIo7IXZlE2
UPbRKrghxzIUGPeNqW0KC+om70TodkoklBIuGawJo8em0ZRkqPXWQM8GRXKUnkr2
q+raryMcoo3LN3aqURC1Xa9YCdmfvAj0JY3H0YQL7cTdX1fKIwcewcVT6SeY1zWl
gXt+FVmHqBGyYttjyf8YzAJqM9e73bq5KKmzRkAiTun4v3HsuktbvxeFG+dw1t4=
=SoRY
-----END PGP SIGNATURE-----

[Bram Moolenaar]

Ingo Karkat wrote:

> On 14-Mar-2015 02:11, James McCoy wrote:
> > On Fri, Mar 13, 2015 at 09:54:18PM +0100, Ingo Karkat wrote:
> >> it's possible to avoid escaping a "[" character:
> >>
> >> ,----[ :help E769 ]---- | When the ']' is not there Vim will not
> >> give an error message but | assume no collection is used. Useful
> >> to search for '['. `----
> >>
> >> But when using that feature in a :substitute command, the
> >> replacement part is mistakenly added to the pattern:
> >>
> >> :s/[//g E486: Pattern not found: [//g
> >
> > No, that's not what's happening. You can leave off the entire
> > replacement and the delimiter before it. When this happens, Vim
> > treats it as deleting the matching strings. To quote:
> >
> > If the {string} is omitted the substitute is done as if it's empty.
> > Thus the matched pattern is deleted. The separator after {pattern}
> > can also be left out then. Example: > :%s/TESTING This deletes
> > "TESTING" from all lines, but only one per line.
>
> Right. My point is that because the "/" delimiters are not actually
> left off (they are there, in the correct, unescaped form), the :s
> command *mistakenly* runs into the case you've quoted. Putting it yet
> another way, the "[" consumes the following characters (including the
> unescaped separators), assuming they belong to the collection, and
> when at the end the collection isn't closed, the parsing should
> backtrack and reinterpret, but it currently doesn't.

I understand that this is confusing, but it's working as documented.
The alternative, detecting the unclosed [ in some circumstances, will
make it less consistent and probably even more confusing. So let's
just leave it as it is. It's clear that if you type the wrong command
things may go wrong. That's what we have undo for.

--
MORTICIAN: What?
CUSTOMER: Nothing -- here's your nine pence.
DEAD PERSON: I'm not dead!
MORTICIAN: Here -- he says he's not dead!
CUSTOMER: Yes, he is.
DEAD PERSON: I'm not!
The Quest for the Holy Grail (Monty Python)

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Ingo Karkat]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I don't see this particular behavior documented. I would expect
something like this (highlighting the current inconsistencies):
"An unclosed [ will do a literal search, but when such pattern is
directly included in a :substitute command (but not when such pattern
is reused from the last search via :s//...), the remainder is
interpreted as the pattern; i.e. you cannot add a replacement part and
:s_flags there.

> The alternative, detecting the unclosed [ in some circumstances,
> will make it less consistent and probably even more confusing. So
> let's just leave it as it is.

I've never argued for that. Of course, the unclosed [ would have to be
detected in _all_ circumstances. That shouldn't be too difficult; the
regexp engines already do that.

> It's clear that if you type the wrong command things may go wrong.

I don't think :s/[/x/g is wrong. It simply parses as pattern=[,
replacement=x, flags=g. Unfortunately, Vim's implementation is buggy
and parses it as pattern=[/x/g replacement= flags= (because the [...]
collection parsing suspends the delimiter parsing until the closing ],
but this isn't a collection, so it shouldn't apply!)

If I had wanted that, I would rather type :s/[\/x\/g (or
:s/[\/x\/g//), i.e. properly escape the / delimiters.

> That's what we have undo for.

As I said, I'm mostly worried about plugins blindly putting @/ into
the :substitute (maybe with prepending / appending something else, and
then failing due to the inconsistency caused by the bug. That would be
hard to detect by the user; undo wouldn't help much.

I'm all for consistency, and fixing this bug would IMO increase that,
so that the special case of unclosed [ and the :s/// separators
interact in a benign way. I was motivated to report this because one
Vim user was struggling with just this, and went to Stack Overflow to
ask for help.

- -- regards, ingo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)

iQEcBAEBAgAGBQJVBIifAAoJEA7ziXlAzQ/v4TsH+gPD7aPCR98auBnAzamL97QB
kewrhUxGy/VtL3sNq8gfjhK/hHx3TyFT+hmcxEhsKHpHw5XT2WnBnagUVadZ1t6l
JD4yxcdxf2z7Rj5RL5Is4hSP/dCJseYydByZcACaPwdeZJ3YJQz1buRFNTjNF4MU
vf0DBaNi6VIULfaQl2QnWiH3p7B2qcp/qPRkuT7Zt6V9SRRBmUx/GfpRCKMW9BWg
89uHCPV77GqRU8EUawdzq3WnxMEb4PJMZiZVwcxLvm6eBqi8WpCIw9+W/oPWqj+3
j2nwXmOzUybc8rp+Y4zojSLsUuhl/Kr9BVG9xo6h9NN1Eq7uUoMzdS8iBJPC2gE=
=AmaI
-----END PGP SIGNATURE-----

[Andy Wokula]

I think the current behavior is ok (snafu ...).
If you use "[" unescaped, you should know what you are doing.
Because: even with "correct" parsing, the pattern may still change, eg
what if "]" occurs in the replacement part:
:s/[/x]/g

Now the original pattern "[" becomes "[/x]" and the only fix is
:s/\[/x]/g

>> That's what we have undo for.
>
> As I said, I'm mostly worried about plugins blindly putting @/ into
> the :substitute (maybe with prepending / appending something else, and
> then failing due to the inconsistency caused by the bug. That would be
> hard to detect by the user; undo wouldn't help much.
>
> I'm all for consistency, and fixing this bug would IMO increase that,
> so that the special case of unclosed [ and the :s/// separators
> interact in a benign way. I was motivated to report this because one
> Vim user was struggling with just this, and went to Stack Overflow to
> ask for help.
>
> - -- regards, ingo

--

Andy

[Ingo Karkat]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Yeah, realistically, the bug would only be included as a low-prio item
on the todo list, and probably not addressed within this decade :-(
Maybe the neovim project will write a different parser and avoid the
problem. I'm a bit sad about this state of affairs, but on the other
hand, Vim is still incredibly useful (even with this minor bug), so
I'm fine with it.

> If you use "[" unescaped, you should know what you are doing.

Agreed. That's why I'm mostly afraid of plugins breaking due the
inconsistency.

> Because: even with "correct" parsing, the pattern may still change,
> eg what if "]" occurs in the replacement part: :s/[/x]/g

No, that's not an unclosed [ followed by ] in the replacement part,
but a correct collection of / and x. The delimiter doesn't have to
escaped inside a collection (didn't find that documented, but it's how
the implementation works).

> Now the original pattern "[" becomes "[/x]" and the only fix is
> :s/\[/x]/g

Right. You can't use unescaped unclosed [ when there's a ] in the
replacement part.

>>> That's what we have undo for.
>>
>> As I said, I'm mostly worried about plugins blindly putting @/
>> into the :substitute (maybe with prepending / appending something
>> else, and then failing due to the inconsistency caused by the
>> bug. That would be hard to detect by the user; undo wouldn't help
>> much.
>>
>> I'm all for consistency, and fixing this bug would IMO increase
>> that, so that the special case of unclosed [ and the :s///
>> separators interact in a benign way. I was motivated to report
>> this because one Vim user was struggling with just this, and went
>> to Stack Overflow to ask for help.
>>
>> - -- regards, ingo
>

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)

iQEcBAEBAgAGBQJVBeE/AAoJEA7ziXlAzQ/vwHQH/joTf1L31UjJy+GkBZ2LQ3/O
NgV5IgtNj6XYEx32sGgRfN4D98giOqHddIcDeBgWD7nWVgnk9h9DD0U9AcVKTjWi
ubQgk/Uuup9xM0Gg3Q5Jt9zsYJgQKiEtzkGfWQL5sXt0KLEBRqEkQ9u3R1ESvl8S
0QyAevIh29m7K9BIhGv12G/MNlhX3mGbkZ53hol/UKse4Rkj6pNrhOy2V5KLoevf
BrItCea6JNw37cJIkkq9SyNxKerYMV5v2A+thh0FI1r8ZyR4p8eM71mXrKZRd6O0
ZdiIjOjUVXLfqTbSbQs70E/iD6UfWS3AzEBcLTg3bYFqlrdDukVUN7EDdRDWzw0=
=Vq7g
-----END PGP SIGNATURE-----