LunarWatcher / auto-pairs

Vim plugin, insert or delete brackets, parentheses, and quotes in pairs
MIT License
173 stars 6 forks source link

Regex issue with new tex multibyte pairs #74

Closed HawkinsT closed 1 year ago

HawkinsT commented 1 year ago

OS: Arch Linux

What vim? (+ version): nvim 0.9 nightly

Describe the bug Since adding the new latex multibyte pairs recently, \( \) and \[ \], there is an issue with, for example, typing \( and then hitting backspace \ [edited to correct bug reproduction method], which will show the error:

Error detected while processing function autopairs#AutoPairsInsert[85] .. autopairs#Insert#checkClose:
line 34:
E54: Unmatched (

This error can be avoided by overriding the new multibyte pair handling with the below, which works as expected:

vim.g.AutoPairs = vim.fn["autopairs#AutoPairsDefine"]({
    { open = [[\\(]], close = "\\)", filetype = "tex" },
    { open = [[\\[]], close = "\\]", filetype = "tex" },
})
LunarWatcher commented 1 year ago

The pairs aren't evaluated as regex anymore. The pairs are processed as if it could be regex, but it appears that escaping results in the total being unescaped and therefore causing errors. The TL;DR: is remove the double escape (I assume [[\\(]] is syntax for a string literal '\\('; you just want '\(' now). The escaping issue remains an annoyance though, but I'll have to whip out a home-made escape solution to fix it. The main problem as your pair definitions stands now is that it wouldn't match \(

That said, you don't need to define those two manually anymore. They're added by default (and those defaults are verified by tests)

HawkinsT commented 1 year ago

Sorry, to explain better, my issue is with the new baked in multibyte pairs you've recently added. My code above fixes the issue by overwriting auto-pairs new baked in multibyte pairs that have this issue. [[\\(]] is equivalent to "\\\\(" (as my config is written in lua). I'll try with a fully minimal config though as I'm surprised the new pairs are passing tests since they don't work for me.

LunarWatcher commented 1 year ago

[[\(]] is equivalent to "\\("

Yes, which is '\\(' (single quotes in Vimscript are raw strings, and it's much easier to talk about raw strings). Regex pairs are disabled by default, which means your \\( is escaped to \\\\( (all raw strings), which still shouldn't be evaluated as regex to yield problems. I can't reproduce in Vim with \\[ or \[ (the first of which only triggers when \\[ is typed, and the latter only triggers when \[ is typed, as expected)

The tests do not run in Neovim, because it's a pain in the ass to set up , but looks like I have to set some up now. My best guess is that Neovim screwed up the regex engine defaults (or the regex engine itself), both of which could be reasons for it to happen.

Unfortunately, the \\[ and \\( tests I did confirms the escape system is working as intended. Unless the regex engine has changed, there's no way the escaping should lead to regex evaluation

HawkinsT commented 1 year ago

Edit (this is wrong): Thanks. I'm not getting this issue on a completely minimal config (only auto-pairs loaded) so I'm now playing around with what may be the cause, sorry, I should have done this initially, but I'll report back here once I work out the issue or how to reproduce it.

HawkinsT commented 1 year ago

My previous comment was incorrect, this error occurs with the following minimal init.lua:

local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
    vim.fn.system({
        "git",
        "clone",
        "--filter=blob:none",
        "https://github.com/folke/lazy.nvim.git",
        "--branch=stable", -- latest stable release
        lazypath,
    })
end
vim.opt.rtp:prepend(lazypath)

require("lazy").setup({
    {
        "LunarWatcher/auto-pairs",
    }
})

but the previous steps to reproduce that I'd listed were incorrect. Instead of typing \(<BS> you need to type \(\ for the error to occur. As before, having the additional config lines fixes the issue, where init.lua becomes:

local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
    vim.fn.system({
        "git",
        "clone",
        "--filter=blob:none",
        "https://github.com/folke/lazy.nvim.git",
        "--branch=stable", -- latest stable release
        lazypath,
    })
end
vim.opt.rtp:prepend(lazypath)

require("lazy").setup({
    {
        "LunarWatcher/auto-pairs",
    }
})

vim.g.AutoPairs = vim.fn["autopairs#AutoPairsDefine"]({
    { open = [[\\(]], close = "\\)", filetype = "tex" },
    { open = [[\\[]], close = "\\]", filetype = "tex" },
})
LunarWatcher commented 1 year ago

Seems like automated tests are off the table. The test framework is incompatible with neovim. Gonna have to poke around manually, which will take a while

HawkinsT commented 1 year ago

No problem, thanks for you help. I'll try to have a further look at it too. At least there is a workaround for now. I assume this issue still isn't present with backslash (rather than backspace as I previously, incorrectly, stated) in vim then?

LunarWatcher commented 1 year ago

Strange, I can reproduce it now. Looks like it's a jump thing

LunarWatcher commented 1 year ago

That should do the trick. I'll also have to look around for other missing escapes, but this particular bug is gone

HawkinsT commented 1 year ago

That fixes it, thanks. It's not ideal behaviour since math mode in latex will almost always contain backslashed command and typing backslash will automatically jump over the closing \), even with a space before the closing pair, e.g. \( | \) (this doesn't occur with my alternative g:AutoPairs command above), but at least it doesn't error now and works as expected.

LunarWatcher commented 1 year ago

Hm, I forgot about that. Might be worth disabling jumping altogether for those pairs? Should be trivial to do. Alternatively, I should be able to set the jump key to ) instead, if that makes more sense.

The only other option is setting b:AutoPairsSearchCloseAfterSpace (the help doc is under g:AutoPairsSearchCloseAfterSpace) manually for the tex filetype, but that has consequences for all pairs, and wouldn't be a default I'd set in auto-pairs itself

HawkinsT commented 1 year ago

Unless the multi-character closing pair of \) can be used I don't think \ or ) are good candidates by themselves as they're both too commonly used in math mode, so perhaps disabling jump for this and the \[ \] pairs would be the best option.

LunarWatcher commented 1 year ago

Yeah, Vim's input system doesn't support multibyte pairs like that. Disabling it is

LunarWatcher commented 1 year ago

Should be disabled now. Also found a minor annoyance with how jump mapping was handled that prevented it from working right away, but whatever. All good now (in theory)