lervag / vimtex

VimTeX: A modern Vim and neovim filetype plugin for LaTeX files.
MIT License
5.53k stars 391 forks source link

Compatibility for builtin syntax groups #1883

Closed clason closed 3 years ago

clason commented 3 years ago

Vimtex 2.0 ships with a new syntax plugin with more consistent and structured syntax and highlight groups; this overrides the built-in syntax file (by default, unless let g:vimtex_syntax_enabled=0 disables the new syntax file and reverts to the old one).

However, it seems that other plugins rely on the specific groups and their names of the original syntax; see #1881 . To enable these, one can manually link the new groups to the old groups with, e.g., hi link texMathZoneZ texMathRegionEnsured.

It would be helpful to add a list of such commands to get back (as close as possible) to the old groups to the Wiki.

Even more convenient would be a function, e.g., vimtex#syntax#compat#set_groups() (or even an option g:vimtex_syntax_compat) that can be used to set these links from your vimrc/init.vim.

lervag commented 3 years ago

For completeness, @Konfekt opened a very related issue #1882. I decided to close that one and continue the discussion here.

... it seems that other plugins rely on the specific groups and their names of the original syntax; see #1881 . To enable these, one can manually link the new groups to the old groups with, e.g., hi link texMathZoneZ texMathRegionEnsured.

It would be helpful to add a list of such commands to get back (as close as possible) to the old groups to the Wiki.

I've started this, see the updated Wiki page here.

Even more convenient would be a function, e.g., vimtex#syntax#compat#set_groups() (or even an option g:vimtex_syntax_compat) that can be used to set these links from your vimrc/init.vim.

Yes, that might be a good solution. Let's first agree on which groups are relevant and necessary, then I'll add this function/option.

Konfekt commented 3 years ago

This linkage as given in the Wiki might restore compatibility with tex-conceal but the function that checks for a math zone by the built-in highlight groups still no longer works:

function! IsMath()
  return match(map(synstack(line('.'), max([col('.') - 1, 1])),
        \ 'synIDattr(v:val, ''name'')'), '^texMathZone\%([A-L]S\?\|[V-Z]\)$') >= 0
endfunction
lervag commented 3 years ago

Hmm. I don't think this trick will work, actually. highlight link only links highlight groups, it does not add syntax matches. So, a syntax item will not be matched by texMathZoneX after hi link ..., if the actual group is texMathRegionX.

With that, I think it seems like the hi link idea does not really give any compatbility benefit. Or am I wrong?

Konfekt commented 3 years ago

Fixed. It needs to follow linked groups by synIDtrans():

  return match(map(synstack(line('.'), max([col('.') - 1, 1])),
        \ 'synIDattr(synIDtrans(v:val), ''name'')'), '^texMathZone\%([A-L]S\?\|[V-Z]\)$') >= 0
lervag commented 3 years ago

Does that also work without the links?

Konfekt commented 3 years ago

Yes!

With that, I think it seems like the hi link idea does not really give any compatbility benefit. Or am I wrong?

At least it solves https://github.com/lervag/vimtex/issues/1880

Konfekt commented 3 years ago

Does that also work without the links?

No, it does not.

lervag commented 3 years ago

My understanding is this:

So, it still seems we are not really fixing anything right now. But I'll be glad to learn I'm wrong!

clason commented 3 years ago

And what about defining the new groups at the syntax level with something like syntax cluster <legacy-group> contains=<vimtex-group>?

lervag commented 3 years ago

I don't think that would work. Clusters don't work that way; they are used as arguments for contains=@cluster or containedin=@cluster.

No, for the desired compatibility, we actually do need to create duplicate syntax rules (if I understand things correctly, and I might not). If I'm right, then this is not something I think is really viable.

Konfekt commented 3 years ago

I agree. Thus, have mercy on the user and keep the syntax group names? (Even though the new names are obviously more pertinent, but backward compatibility is a valuable good.)

lervag commented 3 years ago

Thus, have mercy on the user and keep the syntax group names?

No, sorry, I wont keep the old syntax group names in general. I find the value of consistent group names much higher than the value of "backward" compatibility.

But ok, let's say I did agree to revert the change of Zone to Region, how much would this solve in terms of the actual issues people/you have? If I did this particular change, then I think it would solve your #1880, especially if you simplify your function in the direction I just suggested.

lervag commented 3 years ago

What do you think, @clason?

Konfekt commented 3 years ago

Well, it would solve #1880 and #1881 and could avoid similar issues down the road.

Konfekt commented 3 years ago

That is, without compatibility, all code building on the TeX math syntax group names would have to be doubled from now on.

lervag commented 3 years ago

Well, it would solve #1880 and #1881 and could avoid similar issues down the road.

No, it won't solve #1881, because that is a much more fundamental issue. tex-conceal builds on top of many of the internal tex groups, and I don't think this even includes the Zone based ones. As such, "solving" #1881 is very much out of scope, in my opinion.

That is, without compatibility, all code building on the TeX math syntax group names would have to be doubled from now on.

I don't understand quite what you mean with "all code building". How common is this? I do know of the specific is_mathzone type of functions, but are there more examples?

clason commented 3 years ago

I have no preference for Region over Zone or vice versa -- either is fine for me, although I do prefer the more expressive naming of the new ones (could you tell what those X and Y and Z were standing for in the texMathZones?)

@Konfekt Note that even if the names are kept, the groups themselves are different now so tex-conceal might still break (in more subtle ways).

And I strongly agree that the new syntax groups -- no matter their name, although that is a part of it for some groups at least -- are a significant improvement with practical benefits. I don't think you can have it both ways: vimtex's improved syntax highlighting and full backward compatibility. Luckily, it's easy to choose between the two with the mentioned option so you can keep using both vimtex (for everything else) and tex-conceal (based on the legacy syntax file). So there's no doubling needed.

So I guess there's not much to do here after all...

Konfekt commented 3 years ago

I don't understand quite what you mean with "all code building". How common is this? I do know of the specific is_mathzone type of functions, but are there more examples?

I do not know of any more examples. All code building on it just wanted to emphasize that it is not known which code builds on it.

But ok, let's say I did agree to revert the change of Zone to Region, how much would this solve in terms of the actual issues people/you have? If I did this particular change, then I think it would solve your #1880, especially if you simplify your function in the direction I just suggested.

This simplifcation would be much appreciated.

lervag commented 3 years ago

I'm implementing this change now and pushing it with a version bump to 2.1; the change is documented also in the release notes.

clason commented 3 years ago

Should I adapt the Wiki, too?

lervag commented 3 years ago

Ah, yes please (and thanks for the reminder; I would have forgot about that!).

clason commented 3 years ago

done!

Konfekt commented 3 years ago

Could concealing for \mathbf and \mathrm be added? This used to be delegated to the now (for the new vimtex syntax) obsolete https://github.com/KeitaNakamura/tex-conceal.vim as discussed at https://github.com/lervag/vimtex/issues/557

lervag commented 3 years ago

Yes, I think that should be quite straightforward. I'll look into it when I get the time.