Closed binyomen closed 2 years ago
Unsure if this will get merged but figured I'd at least put up an attempt at a simple fix.
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 link
s but not the hi def link
s? Removing the hi clear
in base16-one-light did fix my issue (as would removing hi link
in typescript-vim.
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.
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.
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.
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
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[^random], 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 e95a02029a4d0df42212ffec10a43c8f50ecc595 is because they used to be default links created with hi def link
and now they're non-default links created with hi link
.
[^random]: I checked out one other theme to see if it does it, and it does.
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.
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.
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.
This version of neovim has the same typescript support as vim 8 patch-8.1.1486, so we should override it as well.