Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compatibility for builtin syntax groups #1883

Closed
clason opened this issue Dec 5, 2020 · 24 comments
Closed

Compatibility for builtin syntax groups #1883

clason opened this issue Dec 5, 2020 · 24 comments

Comments

@clason
Copy link
Contributor

clason commented Dec 5, 2020

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
Copy link
Owner

lervag commented Dec 5, 2020

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
Copy link
Contributor

Konfekt commented Dec 5, 2020

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
Copy link
Owner

lervag commented Dec 5, 2020

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
Copy link
Contributor

Konfekt commented Dec 5, 2020

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
Copy link
Owner

lervag commented Dec 5, 2020

Does that also work without the links?

@Konfekt
Copy link
Contributor

Konfekt commented Dec 5, 2020

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 #1880

@Konfekt
Copy link
Contributor

Konfekt commented Dec 5, 2020

Does that also work without the links?

No, it does not.

@lervag
Copy link
Owner

lervag commented Dec 5, 2020

My understanding is this:

  • With synIDtrans, your function will work with Vimtex v2 as long as you also include the high link linkes suggested in the wiki.
  • With synIDtrans, your function will not work with the built-in syntax plugin.

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

@clason
Copy link
Contributor Author

clason commented Dec 5, 2020

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

@lervag
Copy link
Owner

lervag commented Dec 5, 2020

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
Copy link
Contributor

Konfekt commented Dec 5, 2020

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
Copy link
Owner

lervag commented Dec 5, 2020

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
Copy link
Owner

lervag commented Dec 5, 2020

What do you think, @clason?

@Konfekt
Copy link
Contributor

Konfekt commented Dec 5, 2020

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

@Konfekt
Copy link
Contributor

Konfekt commented Dec 5, 2020

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

@lervag
Copy link
Owner

lervag commented Dec 5, 2020

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
Copy link
Contributor Author

clason commented Dec 5, 2020

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
Copy link
Contributor

Konfekt commented Dec 6, 2020

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
Copy link
Owner

lervag commented Dec 7, 2020

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.

@lervag lervag closed this as completed Dec 7, 2020
lervag added a commit that referenced this issue Dec 7, 2020
@clason
Copy link
Contributor Author

clason commented Dec 7, 2020

Should I adapt the Wiki, too?

@lervag
Copy link
Owner

lervag commented Dec 7, 2020

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

@clason
Copy link
Contributor Author

clason commented Dec 7, 2020

done!

@Konfekt
Copy link
Contributor

Konfekt commented Feb 22, 2021

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 #557

@lervag
Copy link
Owner

lervag commented Feb 22, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants