fixed the issue with the matchit plugin

[ivangeorgiew]

I don't really know how it happens, but in some cases the filetype is set multiple times and the code that sets b:match_words executes for each instance. This leads to the b:match_words being duplicated, when the number of duplicates reaches 10, there is a regex error that there are beyond 10 references and '%' breaks. You can see how I fixed this in the PR

[snoblenet]

Thanks @ivangeorgiew -- I hope this gets merged.

Further discussion here: chrisbra/matchit#11

[VincentCordobes]

I confirm this PR fixes a big issue with the %.
Thanks @ivangeorgiew

FYI, the filetype is often set on events BufNewFile,BufRead (https://github.com/mxw/vim-jsx/blob/master/ftdetect/javascript.vim#L35)
And in my case Neoformat also set the filetype let &filetype = ... while formatting the buffer

[chrisbra]

FWIW, the official javascript ftplugin from the Vim upstream repository does not setup b:match_words. So if no other plugin sets it, it might even be more efficient/readable to just use:

diff --git a/after/ftplugin/jsx.vim b/after/ftplugin/jsx.vim
index f9329fc..6ff94d9 100644
--- a/after/ftplugin/jsx.vim
+++ b/after/ftplugin/jsx.vim
@@ -9,11 +9,8 @@
 " modified from html.vim
 if exists("loaded_matchit")
   let b:match_ignorecase = 0
-  let s:jsx_match_words = '(:),\[:\],{:},<:>,' .
+  let b:match_words = '(:),\[:\],{:},<:>,' .
         \ '<\@<=\([^/][^ \t>]*\)[^>]*\%(/\@<!>\|$\):<\@<=/\1>'
-  let b:match_words = exists('b:match_words')
-    \ ? b:match_words . ',' . s:jsx_match_words
-    \ : s:jsx_match_words
 endif

 setlocal suffixesadd+=.jsx

[chrisbra]

Oh, and BTW, instead of \( use \%(, so that E872 can be avoided (too many \(...) groups)

[VincentCordobes]

I agree! Actually it was like that before #142.

(note that pangloss/vim-javascript doesn't define b:match_words either)

[igemnace]

FWIW, the official javascript ftplugin from the Vim upstream repository does not setup b:match_words. So if no other plugin sets it, it might even be more efficient/readable to just use:

diff --git a/after/ftplugin/jsx.vim b/after/ftplugin/jsx.vim
index f9329fc..6ff94d9 100644
--- a/after/ftplugin/jsx.vim
+++ b/after/ftplugin/jsx.vim
@@ -9,11 +9,8 @@
 " modified from html.vim
 if exists("loaded_matchit")
   let b:match_ignorecase = 0
-  let s:jsx_match_words = '(:),\[:\],{:},<:>,' .
+  let b:match_words = '(:),\[:\],{:},<:>,' .
         \ '<\@<=\([^/][^ \t>]*\)[^>]*\%(/\@<!>\|$\):<\@<=/\1>'
-  let b:match_words = exists('b:match_words')
-    \ ? b:match_words . ',' . s:jsx_match_words
-    \ : s:jsx_match_words
 endif

 setlocal suffixesadd+=.jsx

True, the original code looks much nicer. However, because of how packages are loaded, vim-jsx's after/ftplugin/jsx.vim gets sourced after and thus overrides even user-configured scripts (e.g. $HOME/.vim/after/ftplugin/javascript.vim), not just the official shipped ones in the Vim repo.

Nevertheless, going by my Vim dotfiles history, I've apparently run into this issue as well some time after #142 was merged. I don't understand how exactly the filetype gets set multiple times, but it does cause b:match_words to grow without bounds. A guard, like the one in this PR, looks like it solves the issue.

[chrisbra]

I don't even understand why an official filetype plugin needs to make use of the after directory at all 😕

A guard, like the one in this PR, looks like it solves the issue.

Better would be a guard, that actually checks the content of the b:match_words variable, but the suggested change should also work. However it looks like this plugin is actually unmaintained and @mxw is not interested in merging patches in (I noticed there are already several other issues/PRs about matchit).

[igemnace]

I don't even understand why an official filetype plugin needs to make use of the after directory at all

Fair point, that's pretty much where my gripe originates from. Besides, the javascript.jsx filetype might already ensure vim-jsx files are sourced after javascript files.

However it looks like this plugin is actually unmaintained and @mxw is not interested in merging patches in (I noticed there are already several other issues/PRs about matchit).

Oh, that's a shame. Seems like improvements now live on in individual forks.