Override neovim 0.5.0 and up's typescript support #193

binyomen · 2022-03-06T04:44:31Z

This version of neovim has the same typescript support as vim 8
patch-8.1.1486, so we should override it as well.

This version of neovim has the same typescript support as vim 8 patch-8.1.1486, so we should override it as well.

binyomen · 2022-03-06T04:45:08Z

#184

Unsure if this will get merged but figured I'd at least put up an attempt at a simple fix.

leafgarland

Thanks for looking at this.

richyliu · 2022-04-21T00:06:53Z

This change broke my syntax highlighting. I am on neovim 0.7.0 and using base16-one-light theme. When I open a typescript file, it has no syntax highlighting. Strangely enough, if I run :echo map(synstack(line('.'), col('.')), 'synIDattr(v:val, "name")'), I see the correct syntax highlighting groups. Even stranger is that I can "fix" the syntax highlighting by editing the file again (running :e)

Minimal vimrc:

call plug#begin(stdpath('data') . '/plugged')

Plug 'chriskempson/base16-vim'        " Base16 color scheme
Plug 'leafgarland/typescript-vim'     " Typescript syntax

call plug#end()

colorscheme base16-one-light

Typescript file:

class Foo {}

Result of echo map(synstack(line('.'), col('.')), 'synIDattr(v:val, "name")') on the first letter: ['typescriptReserved'].

After some further investigation into base16 colorscheme, I found that it resets the highlight syntax with hi clear. Perhaps this resets the hi links but not the hi def links? Removing the hi clear in base16-one-light did fix my issue (as would removing hi link in typescript-vim.

binyomen · 2022-04-21T00:50:53Z

Does this only happen when you open a TypeScript file on startup (nvim test.ts) or when you edit one for the first time with something like :edit?

What stands out to me is that I feel like hi clear should work fine assuming the color scheme is set before the syntax file is sourced. I'm wondering if opening the TS file initially is resulting in the syntax being sourced before the color scheme is set. nvim test.ts --startuptime startup.txt might give some hints about load order.

richyliu · 2022-04-21T15:42:46Z

This only happens when I open a ts file on startup. If I open another typescript file within neovim, the problem goes away.

It looks like the typescript file is being sourced after the base16 colorscheme:



times in msec
 clock   self+sourced   self:  sourced script
 clock   elapsed:              other lines

000.009  000.009: --- NVIM STARTING ---
000.798  000.789: locale set
001.242  000.444: inits 1
001.260  000.018: window checked
001.376  000.116: parsing arguments
004.104  002.728: expanding arguments
004.140  000.035: inits 2
004.878  000.739: init highlight
004.881  000.003: waiting for UI
008.488  003.607: done waiting for UI
008.506  000.018: init screen for UI
008.530  000.024: init default mappings
008.577  000.047: init default autocommands
009.182  000.077  000.077: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/ftplugin.vim
009.299  000.047  000.047: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/indent.vim
011.431  002.022  002.022: sourcing /Users/richard/.local/share/nvim/site/autoload/plug.vim
012.840  000.089  000.089: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/filetype.lua
019.064  000.026  000.026: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/ftdetect/typescript.vim
019.187  006.296  006.270: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/filetype.vim
019.268  000.013  000.013: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/ftplugin.vim
019.338  000.011  000.011: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/indent.vim
019.604  000.126  000.126: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/syntax/synload.vim
019.710  000.315  000.188: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/syntax/syntax.vim
026.803  006.984  006.984: sourcing /Users/richard/.local/share/nvim/plugged/base16-vim/colors/base16-one-light.vim
026.828  017.491  001.761: sourcing init.vim
026.841  000.649: sourcing vimrc file(s)
028.038  000.261  000.261: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/gzip.vim
028.098  000.016  000.016: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/health.vim
028.190  000.056  000.056: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/man.vim
028.932  000.195  000.195: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/pack/dist/opt/matchit/plugin/matchit.vim
029.068  000.840  000.646: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/matchit.vim
029.260  000.153  000.153: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/matchparen.vim
029.639  000.342  000.342: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/netrwPlugin.vim
030.390  000.175  000.175: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/autoload/remote/host.vim
030.692  000.176  000.176: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/autoload/remote/define.vim
030.765  000.911  000.560: sourcing /Users/richard/.local/share/nvim/rplugin.vim
030.770  001.082  000.171: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/rplugin.vim
030.896  000.080  000.080: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/shada.vim
030.968  000.027  000.027: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/spellfile.vim
031.136  000.125  000.125: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/tarPlugin.vim
031.275  000.095  000.095: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/tohtml.vim
031.341  000.024  000.024: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/tutor.vim
031.521  000.140  000.140: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/plugin/zipPlugin.vim
032.098  002.017: loading rtp plugins
032.204  000.106: loading packages
032.206  000.001: loading after plugins
032.215  000.009: inits 3
040.329  008.114: reading ShaDa
044.496  000.208  000.208: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/compiler/typescript.vim
044.537  000.516  000.308: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/ftplugin/typescript.vim
044.784  000.140  000.140: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/ftplugin/typescript.vim
045.797  000.269  000.269: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/indent/typescript.vim
046.036  000.189  000.189: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/indent/typescript.vim
047.317  001.147  001.147: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/syntax/typescript.vim
047.576  000.170  000.170: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/syntax/typescript.vim
047.993  000.039  000.039: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/compiler/typescript.vim
048.022  000.127  000.088: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/ftplugin/typescript.vim
048.125  000.008  000.008: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/ftplugin/typescript.vim
048.858  000.026  000.026: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/indent/typescript.vim
048.913  000.009  000.009: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/indent/typescript.vim
049.664  000.633  000.633: sourcing /Users/richard/.local/share/nvim/plugged/typescript-vim/syntax/typescript.vim
049.733  000.011  000.011: sourcing /opt/homebrew/Cellar/neovim/0.7.0/share/nvim/runtime/syntax/typescript.vim
049.849  006.275: opening buffers
049.862  000.013: BufEnter autocommands
049.864  000.002: editing files in windows
049.933  000.069: VimEnter autocommands
049.934  000.001: UIEnter autocommands
049.935  000.001: before starting main loop
050.162  000.227: first screen update
050.163  000.001: --- NVIM STARTED ---

It's a very strange issue. I can't seem to figure it out.

binyomen · 2022-04-23T21:25:20Z

Thanks! The color scheme is being sourced first, but I wonder if it's being set after (in your minimal config with colorscheme). I also get some weirdness with some color schemes when I set the color scheme after having loaded syntax for a file. You can maybe test this out by setting FileType and ColorScheme autocommands at the top of your vimrc and printing in each of them?

I also had some (different) issues with syntax highlighting, but after switching to packer as a plugin manager they seemed to go away. I think this is because packer uses the built-in package support in vim and loads my colorscheme config (under start) before my filetype config (under opt).

Does doing Plug 'leafgarland/typescript-vim', { 'for': 'typescript' } result in it loading later? There's almost certainly a better way, but I'm not super familiar with vim plug. Maybe putting colorscheme base16-one-light immediately after the Plug line that loads it will do stuff in the right order? That's basically what my packer setup does I think: set the color scheme immediately after loading it.

richyliu · 2022-04-25T02:27:55Z

I couldn't find a way to get the colorscheme to be sourced before the ftdetect. I've given up debugging this since it seems that sourcing the colorscheme manually before everything else works:

source ~/.local/share/nvim/plugged/base16-vim/colors/base16-one-light.vim

wincent · 2023-10-07T12:52:42Z

This change broke my syntax highlighting.

👋🏻 I ran into some similar breakage and git bisect led me here.

In my case, I have some overengineered garbage in my dotfiles that uses :ownsyntax on and :ownsyntax off to turn syntax highlighting on and off as windows focus and blur, and I thought that might be a necessary condition to trigger the breakage. But after commenting it all out, I saw that highlighting of TypeScript files is still broken, and it's my use of Base16, which does a :hi clear, that's causing all the highlights to be cleared similar to how @richyliu commented above.

I can see in the --startuptime output that the syntax/typescript.vim is being sourced first, and then the color scheme comes along and clears the highlighting.

Now, I don't know whether :hi clear is a legit thing for a colorscheme to do¹, but :h highlight-clear says:

                                                highlight-clear :hi-clear
:hi[ghlight] clear      Reset all highlighting to the defaults.  Removes all
                        highlighting for groups added by the user.                                                                                                                                          Uses the current value of 'background' to decide which
                        default colors to use.
                        If there was a default link, restore it. :hi-link

and that makes it pretty clear that the reason why the :hi clear is blowing away highlights that were fine before e95a020 is because they used to be default links created with hi def link and now they're non-default links created with hi link.

So with that context, I'm trying to understand the intent of the if that's been in the codebase ever since the root commit of this repository, and which I don't really get in its original form from 12 years ago or in its current form today: 🤔

if version >= 508 || !exists("did_typescript_syn_inits")
  if version < 508 || has('patch-8.1.1486') || has('nvim-0.5.0')
    let did_typescript_syn_inits = 1
    command -nargs=+ HiLink hi link <args>
  else
    command -nargs=+ HiLink hi def link <args>
  endif
  " ... etc

It seems to be "don't rerun the link commands if you've already run them (as indicated by did_typescript_syn_inits). We then have that nested if that says, make "default" links (hi def link), but not when you're on a semi-recent Neovim version, in which case it does hi link.

:h :highlight-default explains:

                                        :hi-default :highlight-default
The [default] argument is used for setting the default highlighting for a
group.  If highlighting has already been specified for the group the command
will be ignored.  Also when there is an existing link.

Using [default] is especially useful to overrule the highlighting of a
specific syntax file.  For example, the C syntax file contains:

        :highlight default link cComment Comment

If you like Question highlighting for C comments, put this in your vimrc file:

        :highlight link cComment Question

Without the "default" in the C syntax file, the highlighting would be
overruled when the syntax file is loaded.

So, on Neovim, we're now getting non-default highlight links, which is why they get blown away by :hi clear.

If I grok the conditionals correctly, we're using def so as to not error for missing groups, and also not override user overrides. Is that right? And in the branch where we're not using def, it's because we want to forceably override Neovim's own terrible highlighting? But if so, I'm confused, because it seems to do override just fine with def too. 🤷🏻 I also don't get why we would set did_typescript_syn_inits only for one branch in that conditional. That seems like a bug? 🤷🏻 And I also don't quite understand the value of not rerunning the hi def link commands.

In short, I have more questions than answers 🤣.

For now, my work around is going to be to apply a small patch to my local copy of typescript-vim (to always use :hi def link, unconditionally), but I'm wondering why we wouldn't just do:

if version < 508
  command -nargs=+ HiLink hi link <args>
elseif version >= 508
  command -nargs=+ HiLink hi def link <args>
endif

Out of curiosity, I went to see what another popular filetype plugin does which overrides styles set by the default bundled with Vim/Neovim's $VIMRUNTIME, vim-markdown, and I see it unconditionally uses hi def link:

hi def link markdownH1                    htmlH1
hi def link markdownH2                    htmlH2
hi def link markdownH3                    htmlH3
hi def link markdownH4                    htmlH4
hi def link markdownH5                    htmlH5
hi def link markdownH6                    htmlH6
" etc...

And if we look at a popular alternative, plasticboy/vim-markdown, that also favors hi def link:

" don't use standard HiLink, it will not work with included syntax files
if v:version < 508
  command! -nargs=+ HtmlHiLink hi link <args>
else
  command! -nargs=+ HtmlHiLink hi def link <args>
endif

which lines up with my suggestion above. Would you be open to a PR for that?

Update: As it was very cheap to do so, sent the PR.

I checked out one other theme to see if it does it, and it does. ↩

Implements the suggestion posted in: leafgarland#193 (comment) Which aligns the plugin with what other popular plugins that override bundled syntax files do. eg. - https://github.com/plasticboy/vim-markdown (uses `hi def link` on version 508 and above) - https://github.com/tpope/vim-markdown (uses `hi def link` unconditionally) This stops highlighting from getting obliterated when activating a colorscheme that uses `:hi clear` (which will reset highlights back to defaults), such as a Base16 theme: - https://github.com/chriskempson/base16-vim/blob/3be3cd82cd31acfcab9a41bad853d9c68d30478d/colors/base16-one-light.vim#L146 or Dracula: - https://github.com/dracula/vim/blob/b2cc39273abbb6b38a3d173d2a5d8c2d1c79fc19/colors/dracula.vim#L19-L24 and presumably others.

leafgarland · 2023-10-08T21:15:46Z

Great writeup @wincent. As you mentioned this logic was there from the original code and this repository started off as a way to make it easier to install Microsoft's own highlight code that they released only in a blog post about a very early version of typescript, so you probably understand more about it than anybody here. I'm happy to go with your PR.

leafgarland · 2023-10-08T21:19:22Z

I'd be happy to see this re-written/cleaned-up to drop some of the crazy old vim support - doubt anyone uses earlier than v7. Unfortunately I probably won't do that as I don't use this code often - I use neovim and tree-sitter highlighters wherever I can.

Override neovim 0.5.0 and up's typescript support

e95a020

This version of neovim has the same typescript support as vim 8 patch-8.1.1486, so we should override it as well.

leafgarland approved these changes Mar 22, 2022

View reviewed changes

leafgarland merged commit f37c425 into leafgarland:master Mar 22, 2022

binyomen deleted the nvim_override branch March 23, 2022 05:45

wincent mentioned this pull request Oct 7, 2023

fix: prefer default links except on ancient Vim versions #203

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Override neovim 0.5.0 and up's typescript support #193

Override neovim 0.5.0 and up's typescript support #193

binyomen commented Mar 6, 2022

binyomen commented Mar 6, 2022

leafgarland left a comment

richyliu commented Apr 21, 2022 •

edited

Loading

binyomen commented Apr 21, 2022 •

edited

Loading

richyliu commented Apr 21, 2022

binyomen commented Apr 23, 2022 •

edited

Loading

richyliu commented Apr 25, 2022

wincent commented Oct 7, 2023 •

edited

Loading

leafgarland commented Oct 8, 2023 •

edited

Loading

leafgarland commented Oct 8, 2023

Override neovim 0.5.0 and up's typescript support #193

Override neovim 0.5.0 and up's typescript support #193

Conversation

binyomen commented Mar 6, 2022

binyomen commented Mar 6, 2022

leafgarland left a comment

Choose a reason for hiding this comment

richyliu commented Apr 21, 2022 • edited Loading

binyomen commented Apr 21, 2022 • edited Loading

richyliu commented Apr 21, 2022

binyomen commented Apr 23, 2022 • edited Loading

richyliu commented Apr 25, 2022

wincent commented Oct 7, 2023 • edited Loading

Footnotes

leafgarland commented Oct 8, 2023 • edited Loading

leafgarland commented Oct 8, 2023

richyliu commented Apr 21, 2022 •

edited

Loading

binyomen commented Apr 21, 2022 •

edited

Loading

binyomen commented Apr 23, 2022 •

edited

Loading

wincent commented Oct 7, 2023 •

edited

Loading

leafgarland commented Oct 8, 2023 •

edited

Loading