rust-lang / rust.vim

Vim configuration for Rust.
Apache License 2.0
3.83k stars 292 forks source link

[Bug] Incorrect angle bracket matching #469

Open alexmozaidze opened 2 years ago

alexmozaidze commented 2 years ago

rust.vim version: latest

It treats the angle brackets of the comparison operators as valid to match on, which is incorrect.

Steps to reproduce:

if 0 < 0 {
} else if 0 > 0 {
}

If we put the cursor on any of the brackets above, it will match.

Expected vs. actual behavior:

Expectation: Use matchpairs defined by the user. Reality: rust.vim decides to do setlocal matchpairs+=<:> for whatever reason.

Debug info:

rust.vim Global Variables:

let g:ftplugin_rust_source_path = v:null
let g:loaded_syntastic_rust_cargo_checker = v:null
let g:loaded_syntastic_rust_filetype = v:null
let g:loaded_syntastic_rust_rustc_checker = v:null
let g:rust_bang_comment_leader = v:null
let g:rust_cargo_avoid_whole_workspace = v:null
let g:rust_clip_command = v:null
let g:rust_conceal = v:null
let g:rust_conceal_mod_path = v:null
let g:rust_conceal_pub = v:null
let g:rust_fold = v:null
let g:rust_last_args = v:null
let b:rust_last_args = []
let g:rust_last_rustc_args = v:null
let b:rust_last_rustc_args = []
let g:rust_original_delimitMate_excluded_regions = v:null
let g:rust_playpen_url = v:null
let g:rust_prev_delimitMate_quotes = v:null
let g:rust_recent_nearest_cargo_tol = v:null
let g:rust_recent_root_cargo_toml = v:null
let g:rust_recommended_style = v:null
let g:rust_set_conceallevel = v:null
let g:rust_set_conceallevel=1 = v:null
let g:rust_set_foldmethod = v:null
let g:rust_set_foldmethod=1 = v:null
let g:rust_shortener_url = v:null
let g:rustc_makeprg_no_percent = v:null
let g:rustc_path = v:null
let g:rustfmt_autosave = 0
let g:rustfmt_autosave_if_config_present = v:null
let g:rustfmt_command = 'rustfmt'
let g:rustfmt_emit_files = 1
let g:rustfmt_fail_silently = 0
let g:rustfmt_options = ''
let g:syntastic_extra_filetypes = v:null
let g:syntastic_rust_cargo_fname = v:null
rustfmt 1.4.38-nightly (71226d7 2022-02-04)

rustc 1.60.0-nightly (71226d717 2022-02-04)

cargo 1.60.0-nightly (25fcb13 2022-02-01)

NVIM v0.6.1
Build type: Release
LuaJIT 2.1.0-beta3
Скомпилирован  builduser

Features: +acl +iconv +tui
See ":help feature-compile"
alexmozaidze commented 2 years ago

Also, matching on angle brackets is the most uncommon thing to do in Rust IMO.

chris-morgan commented 2 years ago

Matching on angle brackets is very common, due to generics, and the false positives are not particularly common. The current behaviour is certainly imperfect, but historically I and multiple others have found it considerably better than not adding <:>.

However, it just occurred to me that we can probably do better with matchit.vim, by ignoring left angle brackets preceded by whitespace as very probably the less-than operator rather than generics:

setlocal matchpairs-=<:>
let b:match_words = " \\@<!<:>"

(You could put this in an after/ftplugin/rust.vim independent of what we decide here, incidentally.)

However, this only works if you have loaded matchit.vim (e.g. packadd! matchit in vimrc), which we can’t depend on. (We used to load it, but that line was removed as part of upstreaming it to the Vim runtime files, and applied here in b5d4cc25122586939df0a5c68391f649f6c83bd8.)

I’m inclined to suggest roughly this for ftplugin/rust.vim (plus a corresponding b:undo_ftplugin change):

-setlocal matchpairs+=<:>
+if exists("loaded_matchit")
+    let b:match_words = " \\@<!<:>"
+else
+    setlocal matchpairs+=<:>
+endif

However, this does come at the cost of no longer highlighting matching generic angle brackets, which I’m not enthusiastic about, but it might be a reasonable cost. I seek others’ opinions.

alexmozaidze commented 2 years ago

I use matching on {}, [] and () because they can be nested and can be indented vertically, using % is better than doing f);;; or 6j. <> usually are not nested (nor are they indented vertically, where keyword exists for that), so doing f> is equivalent of % on one of the brackets. Also, MatchUp plugin shows the position of the matching thing that is off-screen, and having it show up on the screen when I have >= is annoying, I tried disabling it from init.vim but it's no use, I had to fork this plugin in order to remove setlocal matchpairs+=<:>.

chris-morgan commented 2 years ago

I have no intention that we should remove angle bracket matching for generics. I’m open to us improving the heuristic via b:match_words for matchit.vim or equivalent, but it is not being removed: it’s too useful, and false positives not too common.

Angle brackets for generics are regularly nested, though to be sure there are plenty of code bases that will never experience this. Spreading across multiple lines is fairly irrelevant (that’s not all matching is for), but even then I’d say it’s not rare to have generic angle brackets spread across multiple lines, though it’s certainly not common, with where having mostly replaced the place where it was most common.

If you want to revert anything any plugin is doing, look at :help after-directory, which I obliquely mentioned in my previous comment.