rktjmp / lush.nvim

Create Neovim themes with real-time feedback, export anywhere.
MIT License
1.44k stars 47 forks source link

Remove syntax reset #73

Closed mcchrish closed 2 years ago

mcchrish commented 2 years ago

It's been observed that syntax reset is not necessary anymore. Ref: https://github.com/vim/colorschemes/issues/34

With the line, syncolor.vim is loaded twice. If anything it might improve the startup a tiny tiny bit.

rktjmp commented 2 years ago

syncolor.vim isn't in neovim so I don't think we would hit the double-require issue.

For reference:

Syntax highlighting:
  syncolor.vim has been removed. Nvim now sets up default highlighting groups
  automatically for both light and dark backgrounds, regardless of whether or
  not syntax highlighting is enabled. This means that |:syntax-on| and
  |:syntax-enable| are now identical. Users who previously used an
  after/syntax/syncolor.vim file should transition that file into a
  colorscheme. |:colorscheme|
                        *:syntax-reset* *:syn-reset*
If you have changed the colors and messed them up, use this command to get the
defaults back: >

  :syntax reset

It is a bit of a wrong name, since it does not reset any syntax items, it only
affects the highlighting.

Note that the syntax colors that you set in your vimrc file will also be reset
back to their Vim default.
Note that if you are using a color scheme, the colors defined by the color
scheme for syntax highlighting will be lost.

Note that when a color scheme is used, there might be some confusion whether
your defined colors are to be used or the colors from the scheme.  This
depends on the color scheme file.  See |:colorscheme|.
: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|

Seems reasonable to remove, "transition to a colorscheme" → "hi clear removes hi's added by the user[s colorscheme]", seems like the prescribed use.

Have you tested this with zen*? I nothing blew up?

mcchrish commented 2 years ago

Have you tested this with zen*? I nothing blew up?

I have. I use force_clean = false and handle the cleaning part externally. Additionally I added a guard to only hi clear when transitioning to another colorscheme. It help improved the startup by about 1-3ms. It's working fine so far.

syncolor.vim isn't in neovim so I don't think we would hit the double-require issue.

That's interesting. I still see it in 0.5.1 so it must be slated for 0.6 (strangely I don't see it in https://github.com/neovim/neovim/issues/14090 and might not be considered breaking)

rktjmp commented 2 years ago

syncolor.vim isn't in neovim so I don't think we would hit the double-require issue.

That's interesting. I still see it in 0.5.1 so it must be slated for 0.6 (strangely I don't see it in neovim/neovim#14090 and might not be considered breaking)

My fault for not looking there, I only checked my local install.

Sounds good. :+1: