Neovim mode doesn't handle some > and < characters properly about vroom HOT 22 OPEN

@equalsraf Found more escaping issues.

from vroom.

Another case I'm expecting to be an issue is a quote character inside quotes.

Looks like push_keys is supposed to support roughly the usage we need, except that I'm not sure it accepts <> notation like . @tarruda, do you know if push_keys or anything else in the python client will accept <> notation the same way vim --remote-send does?

Alternatively, we could see if we could invoke (n)vim itself to interpret the keys somehow.

from vroom.

Actually vim_push_keys wasn't implemented yet, I think @equalsraf simulated it using vim.command and feedkeys. He's working on it in neovim/neovim#914.

As for parsing special key strings such as <cr>, I don't think it would be a problem since we can simply use the same function that backs feedkeys implementation

from vroom.

@equalsraf Sorry, I think I just deleted your comment somehow and can't figure out how to restore it. Here's a copy (I might have messed up some of the formatting. If so and you want to re-post it, I can just delete this copy):

Here is a big patch for the feedkeys logic - still doesn't fix this issue though - it fixes wrongfully escaping < twice and avoids escaping < inside single quotes. The call issued to neovim for that case

call feedkeys(":execute 'norm' '<Leader>Hh'\<CR>")

Which seems correct. Unfortunately I think --remote-send was the one translating . Time to build something equivalent for the Neovim API :D.

Here is the patch -- apologies in advance for the funky re

diff --git a/vroom/neovim_mod.py b/vroom/neovim_mod.py
index e762c13..eefd02b 100644
--- a/vroom/neovim_mod.py
+++ b/vroom/neovim_mod.py
@@ -3,7 +3,7 @@ from vroom.vim import Communicator as VimCommunicator
 import subprocess
 import time
 import neovim
-import os
+import os, re
 try:
   from StringIO import StringIO
 except ImportError:
@@ -44,6 +44,22 @@ class Communicator(VimCommunicator):
         time.sleep(0.01)
     self.conn = neovim.connect(self.args.servername)

+  @staticmethod
+  def EscapeInput(inp):
+    def replace_angle(m):
+        if not m.group().startswith("'"):
+            return re.sub(r"(?<=[^\\])<|^<", r"\\<", m.group())
+        return m.group()
+
+    # Backslashes
+    inp = inp.replace('\\', '\\\\')
+    # Quotes
+    inp = inp.replace('"', '\\"')
+    # Escape <Expr> unless already escaped or within
+    # single quotes ''
+    inp = re.sub(r"'[^']*'|[^']+", replace_angle, inp)
+    return inp
+
   def Communicate(self, command, extra_delay=0):
     """Sends a command to Neovim

@@ -53,11 +69,10 @@ class Communicator(VimCommunicator):
     Raises:
       Quit: If vim quit unexpectedly.
     """
+    neovimcmd = 'call feedkeys("%s")' % self.EscapeInput(command)
-    command = command.replace('\\', '\\\\')
-    command = command.replace('"', '\\"')
-    command = command.replace('<', '\\<')
-    self.conn.command('call feedkeys("%s")' % command)
+    self.writer.Log(neovimcmd)
+    self.conn.command(neovimcmd)
     self._cache = {}
     time.sleep(self.args.delay + extra_delay)

from vroom.

As for parsing special key strings such as , I don't think it would be a problem since we can simply use the same function that backs feedkeys implementation

@tarruda Replying here as well: feedkeys() does not do any <…> transformations. It is a feature of VimL parser if you use double-quoted strings or :map command family parser. Maybe something else is doing this processing as well, but feedkeys() is not on that list.

from vroom.

@ZyX-I right you are. Apparently I got confused because --remote-send is doing something else. Namely calling replace_termcodes() ... still investigating.

from vroom.

It's hard for me to reason about all these levels of escaping, but couldn't a backslash inside single quotes have issues as well?

I think r"(?<=[^\\])<|^<" can be replaced with r"(?<!\\)<", BTW. And that probably isn't technically right since I think it would get tripped up on things like \\< (which is unescaped but treated as if it's escaped). But as long as we have a plan for fixing it properly, I don't mind a compromise that's slightly less correct and slightly less hideous in the meantime. =)

from vroom.

Mmmm, I got tired of fighting over string escaping and backported the original Vim remote-send as a Neovim API function. @tarruda is something like this acceptable? equalsraf/neovim@dd599a9

from vroom.

Looks good to me!

BTW, we can update vroom as soon as the changes are in the pypi version. This stuff is still experimental status and I have no big concerns about version compatibility.

from vroom.

Looks like the new feedkeys function was merged a few weeks ago. We ready to proceed here?

from vroom.

Actually the saga continues in neovim/neovim#920. I've been pretty busy for the past weeks, sorry for the delay.

from vroom.

No worries. Thanks for the update!

from vroom.

This should be fixed since #45.

from vroom.

I see failures in maktaba from vroom --neovim maktaba/vroom/pluginsignals.vroom that seem to be a result of escaping problems. If I use vroom's --interactive and check :let g:weirdpath for neovim, I see

g:weirdpath            /home/dbarnett/.vim/vim-addons/maktaba/vroom/fakeplugins/
weird¬pâ<80>¦l✓u↓g⏎iâ<80>½n

whereas in vim (using --interactive and adding a :throw 'BOOM' to trigger it) I see the expected

g:weirdpath            /home/dbarnett/.vim/vim-addons/maktaba/vroom/fakeplugins/
weird¬p…l✓u↓g⏎i‽n

I can close this issue and open a separate one if you think it's an unrelated problem, but it looks like we're still having trouble getting exactly the right level of escaping.

from vroom.

Not quite an escaping error, more like a bug - 'replace_termcodes()' did something unexpected with the character … in utf8 this would be \xe2\x80\xa6 but replace_termcodes turned into '\xe2\x80\xfeX\xa6' i.e. it replaced \x80 with \x80\xfeX. This is basically what Vim uses internally for escaping chars - but it is clearly not being well handled by feedkeys()

I'll need to have a deeper look.

from vroom.

Yes, the wrong arguments were being passed to replace_termcodes. The following patch should fix the problem:

diff --git a/vroom/neovim_mod.py b/vroom/neovim_mod.py
index f50ec42..3c6fcb2 100644
--- a/vroom/neovim_mod.py
+++ b/vroom/neovim_mod.py
@@ -52,7 +52,7 @@ class Communicator(VimCommunicator):
       Quit: If vim quit unexpectedly.
     """
     self.writer.Log(command)
-    parsed_command = self.conn.replace_termcodes(command, True, True, True)
+    parsed_command = self.conn.replace_termcodes(command, False, True, False)
     self.conn.feedkeys(parsed_command)
     self._cache = {}
     time.sleep(self.args.delay + extra_delay)

from vroom.

Nevermind the patch above, it fixes only that test because it disables escaping, but fails all others

from vroom.

@tarruda I think the way to go about it is to disable the K_SPECIAL escaping in term.c:replace_termcodes() (only when it is called from the API) - I'll get a PR going shortly.

from vroom.

Looking at it it easier to alter vim_feedkeys to skip escaping.

from vroom.

@tarruda I think the way to go about it is to disable the K_SPECIAL escaping in term.c:replace_termcodes() (only when it is called from the API) - I'll get a PR going shortly

I just looked at replace_termcodes and the code which takes care of skipping multibyte characters seems to be wrong. I have no idea why this works on vim though

from vroom.

@tarruda not quite, replace_termcodes() escapes K_SPECIAL(0x80) in the string once and returns the internal Vim representation BUT vim_feedkeys will escape 0x80 a second time. As far as I can see we can

1. Change vim_replace_termcodes to make escaping of K_SPECIAL optional - this seems tricky in some corner cases (from_part=True)
2. Change vim_feedkeys() to make escaping optional() just add a new parameter to skip vim_strsave_escape_csi()

Both seem to fix the issue

from vroom.

@tarruda not quite, replace_termcodes() escapes K_SPECIAL(0x80) in the string once and returns the internal Vim representation BUT vim_feedkeys will escape 0x80 a second time.

👍

from vroom.