onivim / oni

Oni: Modern Modal Editing - powered by Neovim
https://www.onivim.io
MIT License
11.36k stars 301 forks source link

Can't use externalized syntax highlighting #2552

Open 00sapo opened 5 years ago

00sapo commented 5 years ago

Oni Version: 0.3.6 Neovim Version (Linux only): 0.3.1 Operating System: Manjaro 17.1.12, Linux 4.14.66-1-MANJARO x86_64

Issue: Can't use externalized syntax highlighting

Expected behavior: When I want to use a syntax highlighting provided by some Vim plugin and I disable TextMate syntax, the one coming from the plugin should be used

Actual behavior: Default syntax is shown

Steps to reproduce:

Use SemanticHighlight plugin:

syntax on
Plug 'jaxbot/semantic-highlight.vim'

Disable TextMate:

"editor.textMateHighlighting.enabled": false,
"oni.loadInitVim": true,

Open a file and launch SemanticHighlight: :SemanticHighlight

In nvim: image

In oni: image

oni-bot[bot] commented 5 years ago

Hello and welcome to the Oni repository! Thanks for opening your first issue here. To help us out, please make sure to include as much detail as possible - including screenshots and logs, if possible.

CrossR commented 5 years ago

I think this may be due to how the plugin checks if a GUI is running. The plugin checks the following to check if it should use GUI colours:

https://github.com/jaxbot/semantic-highlight.vim/blob/01e646ca17dfe0143ef9c8a04a684e8f100bd20d/plugin/semhl.vim#L144

Of which none are set in Oni, due to either not making sense to enable or not existing to enable.

If I manually force the plugin to work in Oni (by adding an exists('g:gui_oni') to the check above), then the plugin works as expected:

after

However, that is obviously not a fix at all. I'm not sure what the general nvim way of doing this is, except for checking nvim_list_uis() and checking the RGB flag it returns (which Oni does set). Perhaps someone more versed in how to do this could jump in and say?

But after a change is made to the plugin, the setup you've got should work as expected. Its maybe a different issue of if Oni should stop the textmate highlights if it detects some new user enabled ones.

CrossR commented 5 years ago

I think this is also the same as Goyo.vim, where the following was recommended:

https://github.com/junegunn/goyo.vim/issues/92#issuecomment-414124059

00sapo commented 5 years ago

You are saying that the best way is to make SemanticHighlight compatible to oni, right? I think that since there are two plugins that don't work because of oni, maybe oni itself could try to make an effort about this issue... But I'm not a Vim developer, so maybe I'm wrong.

CrossR commented 5 years ago

You are saying that the best way is to make SemanticHighlight compatible to oni, right?

Not quite, more just to be compatible with any Neovim GUI, not just Oni. Ie, the current approach means it also doesn't work in nvim-qt, NyaoVim and I assume any other neovim gui that uses GUI colours.

There is a PR that would fix it for some of them (https://github.com/jaxbot/semantic-highlight.vim/pull/61) but it sounds like setting &termiguicolors in Oni isn't really appropriate, as per @justinmk's comment on the Goyo issue.

Using nvim_list_uis() seems to be the way to work for all nvim GUIs from now on, not just Oni. I think since the nvim GUIs (including Oni!) are still mainly fairly early on, some plugins don't react to them very well yet, and also why not many plugins have adapted to them.

badosu commented 5 years ago

You can try:

    "ui.colorscheme"           : "n/a", // Load init.vim colorscheme, remove this line if wants Oni's default

See https://github.com/onivim/oni/wiki/How-To:-Minimal-Oni-Configuration, there are some other options there that might help you

00sapo commented 5 years ago

Ok, for now I have added support to Oni in my fork version of SemanticHighlight. I plan to come back and implement the nvim_list_uis() way.

badosu commented 5 years ago

Shouldn't we make gui_running be true on Oni?

CrossR commented 5 years ago

Shouldn't we make gui_running be true on Oni?

I don't know if that exists in the nvim side of things. I did a :helpgrep gui_running and the only reference I found to it was in the ALE config. If it does, then possibly, but my very basic understanding is that it relates to GVim rather than a GUI in general?

badosu commented 5 years ago

If it does, then possibly, but my very basic understanding is that it relates to GVim rather than a GUI in general?

Yes, also Macvim.

However it seems that this is the case because they were the only GUIs available at the time. I think it would be sensible to use this as many plugins may use this variable to detect capabilities, e.g. instead of using t_Co detecting if it is a GUI, etc..

I am not an expert in this area so I cannot say for sure, but taking a look at @00sapo's patch on semantichighlighting this would be avoided. And perhaps for many other plugins the behavior would be more consistent.

justinmk commented 5 years ago

Well, we made a choice in Nvim to not restore gui_running because multiple UIs may be attached.

In practice that choice has so far been rather pedantic, but OTOH--also "in practice"--it hasn't turned out to be that important. The most egregious uses of gui_running were colorschemes, which have no business checking the type of UI anyways (ctermbg and guibg should both be set, let Nvim decide which one to use).

With 'termguicolors' , it makes even less sense for many of these plugins to make the often wrong assumptions that they did based on gui_running. And as it happens, the plugin that inspired this request has had a request to check termguicolors for 2 years, no one seems to have noticed that the author no longer maintains any of their vim plugins :)

badosu commented 5 years ago

@justinmk Thanks for the feedback!