onedarkpro.nvim icon indicating copy to clipboard operation
onedarkpro.nvim copied to clipboard

Don't rely on bufenter or similar to apply filetype_hlgroups

Open petobens opened this issue 1 year ago • 24 comments

Hi, consider the following minimal.lua file:

local fn = vim.fn

-- Ignore default config and plugins and define new dirs
local test_dir = '/tmp/nvim-minimal'
vim.opt.runtimepath:remove(fn.expand('~/.config/nvim'))
vim.opt.packpath:remove(fn.expand('~/.local/share/nvim/site'))
vim.opt.runtimepath:append(fn.expand(test_dir))
vim.opt.packpath:append(fn.expand(test_dir))

-- Install packer
local install_path = test_dir .. '/pack/packer/start/packer.nvim'
if fn.empty(fn.glob(install_path)) > 0 then
    packer_bootstrap = fn.system({
        'git',
        'clone',
        '--depth',
        '1',
        'https://github.com/wbthomason/packer.nvim',
        install_path,
    })
    vim.cmd([[packadd packer.nvim]])
end

-- Setup packer
local packer = require('packer')
packer.init({
    package_root = test_dir .. '/pack',
    compile_path = test_dir .. '/plugin/packer_compiled.lua',
})
packer.startup(function(use)
    -- Packer can manage itself
    use('wbthomason/packer.nvim')

    -- Plugins
    use({ 'nvim-treesitter/nvim-treesitter', run = ':TSUpdate' })
    use('olimorris/onedarkpro.nvim')
    use({
        'nvim-telescope/telescope.nvim',
        requires = {
            'nvim-lua/plenary.nvim',
        },
    })

    -- Auto install plugins
    if packer_bootstrap then
        packer.sync()
    end
end)

-- Plugin setup
local ok, treesitter = pcall(require, 'nvim-treesitter.configs')
if ok then
    treesitter.setup({
        highlight = {
            enable = true,
        },
        ensure_installed = {
            'markdown',
        },
    })
end

local ok, onedarkpro = pcall(require, 'onedarkpro')
if ok then
    local palette = {
        dark_red = '#be5046',
    }
    local p = palette
    onedarkpro.setup({
        theme = 'onedark',
        colors = palette,
        filetype_hlgroups = {
            markdown = {
                markdownTSPunctSpecial = { fg = p.dark_red, style = 'bold' },
            },
        },
    })

    onedarkpro.load()
end

Now, as in the GIF, use :Telescope find_files on a directory where you have a markdown file (in my case ~/Desktop with foo.md as:

# Foo

## Bar

You will see that Telescope's preview despite relying on treesitter doesn't highlight the markdown sections as per filetype_hlgroups override. In general I've noticed that the hlgroup override occur on Buf/WindowEnter (my second GIF)

onedark

onedark2

petobens avatar Aug 06 '22 19:08 petobens

Hello @petobens 👋🏼. Yes this is a documented shortfall of the filetype highlights. I've not found a way to highlight the telescope preview unfortunately. Think something may need to change on the Telescope side. Have you found it working elsewhere?

Would welcome pull requests on this!

olimorris avatar Aug 06 '22 20:08 olimorris

Hi @olimorris . This is not strictly related to Telescope but (I believe) to autocmds. For instance if you add the following lines to my previous minimal.lua file:

local session_acg = vim.api.nvim_create_augroup('session', { clear = true })
vim.api.nvim_create_autocmd('VimLeavePre', {
    group = session_acg,
    command = [[execute 'mksession! ~/Desktop/test_session.vim']],
})

and then, as in the GIF, do:

  1. Open nvim and edit two files in a vertical split foo.txt and foo.md (you'll se here that the filetype highlights are correctly applied)
  2. Move to the foo.txt file and then press qall to exit nvim (make sure that when exiting your cursor is on the foo.txt file
  3. Open nvim again loading the minimal.lua file and run :source ~/Desktop/test_session.vim

After 3. you will see that the markdown file is not correctly highlighted unless we move the cursor to the markdown window/buffer.

onedark_md

petobens avatar Aug 07 '22 03:08 petobens

I am in the middle of a large rewrite (see the latest pull request) and am now implementing filetype_highlights again so will look at adding a test case for this.

Could you try adding more events to the autocmd and see if that changes anything?

olimorris avatar Aug 07 '22 07:08 olimorris

Hey @petobens. Could you look at this again now the rewrite is complete?

olimorris avatar Aug 16 '22 14:08 olimorris

Hi! Can you point to where should I add the autocmds?

petobens avatar Aug 16 '22 20:08 petobens

They're in the ft_highlight.lua file.

olimorris avatar Aug 16 '22 21:08 olimorris

Mmm I played around a bit with this (by adding BufAdd events,etc) but without success. There seems to be something resetting the highlight upon leaving the buffer as per the following GIF. I can take a close look over the weekend but dunno what's going on:

Peek 2022-08-16 22-24

petobens avatar Aug 17 '22 01:08 petobens

So the basic premise of filetype highlights is that when you enter a buffer which has a filetype which the user has defined as having filetype highlights, then it applies them. If the user navigates to a buffer which doesn't have any user defined filetypes then it reverts to the theme's default highlights. That way you're not applying lua specific highlights to a markdown file.

It does all of this by creating highlight namespaces (a new addition in Neovim 0.7) which allows me to create namespaces for the theme's default highlights and then for any custom filetype highlights. It's then easy to switch between the namespaces depending on the filetype.

olimorris avatar Aug 17 '22 08:08 olimorris

The user navigates to a buffer which doesn't have any user defined filetypes then it reverts to the theme's default highlights

Can this behaviour be changed? I see this more like a bug rather than a feature. To illustrate my point, consider the following minimal.lua file:

local fn = vim.fn

-- Ignore default config and plugins and define new dirs
local test_dir = '/tmp/nvim-minimal'
vim.opt.runtimepath:remove(fn.expand('~/.config/nvim'))
vim.opt.packpath:remove(fn.expand('~/.local/share/nvim/site'))
vim.opt.runtimepath:append(fn.expand(test_dir))
vim.opt.packpath:append(fn.expand(test_dir))

-- Install packer
local install_path = test_dir .. '/pack/packer/start/packer.nvim'
if fn.empty(fn.glob(install_path)) > 0 then
    packer_bootstrap = fn.system({
        'git',
        'clone',
        '--depth',
        '1',
        'https://github.com/wbthomason/packer.nvim',
        install_path,
    })
    vim.cmd([[packadd packer.nvim]])
end

-- Setup packer
local packer = require('packer')
packer.init({
    package_root = test_dir .. '/pack',
    compile_path = test_dir .. '/plugin/packer_compiled.lua',
})
packer.startup(function(use)
    -- Packer can manage itself
    use('wbthomason/packer.nvim')

    -- Plugins
    use({ 'nvim-treesitter/nvim-treesitter', run = ':TSUpdate' })
    use('olimorris/onedarkpro.nvim')

    -- Auto install plugins
    if packer_bootstrap then
        packer.sync()
    end
end)

-- Plugin setup
local ok, treesitter = pcall(require, 'nvim-treesitter.configs')
if ok then
    treesitter.setup({
        highlight = {
            enable = true,
        },
        ensure_installed = { 'bash', 'markdown' },
    })
end

local ok, onedarkpro = pcall(require, 'onedarkpro')
if ok then
    local p = {
        fg = '#abb2bf',
        dark_red = '#be5046',
    }
    onedarkpro.setup({
        theme = 'onedark',
        colors = p,
        ft_highlights = {
            markdown = {
                markdownTSPunctSpecial = { fg = p.dark_red, style = 'bold' },
            },
            sh = {
                bashTSParameter = { fg = p.fg },
            },
        },
    })
    onedarkpro.load()
end

If you then open a markdown and bash file and switch the cursor between one window and the other you can see, as in the GIF, that colors are constantly changing.

multicolor

So to this point:

the user navigates to a buffer which doesn't have any user defined filetypes then it reverts to the theme's default highlights

Can you change the behaviour such that if a user has defined a filetype highlight then it applies such behaviour even when switching to another buffer/window/file?

Thanks in advance and sorry for the late reply.

petobens avatar Aug 24 '22 03:08 petobens

Can you checkout the develop branch?

You'll need to add:

ft_highlights_force = true

to your setup function.

olimorris avatar Aug 24 '22 06:08 olimorris

I've tried the develop branch with the minimal.lua in my previous post (and adding the new force setting) and still get the multicolor change when switching windows. Can you reproduce it?

petobens avatar Aug 24 '22 21:08 petobens

I've tried the develop branch with the minimal.lua in my previous post (and adding the new force setting) and still get the multicolor change when switching windows. Can you reproduce it?

Even with ft_highlights_force = true? I have it working in my config and have added a test too

olimorris avatar Aug 25 '22 06:08 olimorris

If I open two files with overrides (as in my Gif above with the markdown and bash files) then it doesn't work. If I open just one file with an override (say the markdown file and a lua file) then it works.

petobens avatar Aug 25 '22 11:08 petobens

If I open two files with overrides (as in my Gif above with the markdown and bash files) then it doesn't work

So you're saying that the highlight groups for Markdown (markdownTSPunctSpecial) should remain when you're in the Bash (bashTSParameter) file!

I could create a namespace called "all" where all of a users custom filetype highlights go. The only issue would be if you had Python and Ruby filetype highlights that both defined the TSFunction highlight. As there would be no way of knowing what should be applied.

olimorris avatar Aug 25 '22 12:08 olimorris

So you're saying that the highlight groups for Markdown (markdownTSPunctSpecial) should remain when you're in the Bash (bashTSParameter) file!

Yep, I don't want the editor to become a rainbow of colors when switching buffers :)

I could create a namespace called "all" where all of a users custom filetype highlights go. The only issue would be if you had Python and Ruby filetype highlights that both defined the TSFunction highlight. As there would be no way of knowing what should be applied.

Dunno about the implementation (i thought that maybe this could be implemented with autocmds). My main point (the way I see it) is that each filetype should have the ability to overrides higlighlights in a permanent way.

petobens avatar Aug 25 '22 21:08 petobens

I can't see how it can work with an autocmd as everything is namespaced but I think this may be easy to solve. I'll have a play this weekend.

olimorris avatar Aug 25 '22 22:08 olimorris

Awesome. Thanks!!

petobens avatar Aug 25 '22 22:08 petobens

@petobens - Okay...I've had a quick attempt. Let me know if this comes anywhere close to working. I'm not sure there is an eloquent way to solve this.

olimorris avatar Aug 25 '22 22:08 olimorris

Do I need to add some extra config? With the minimal.lua config + the ft_highlights_force = true, setting it still not working for me.

petobens avatar Aug 25 '22 22:08 petobens

If you put your desired filetype highlights in highlights instead of ft_highlights, would that actually solve it? Assuming there are not many groups you wish to overwrite.

olimorris avatar Aug 26 '22 08:08 olimorris

If you put your desired filetype highlights in highlights instead of ft_highlights, would that actually solve it? Assuming there are not many groups you wish to overwrite.

That would probably defeat the purpose of having ft_highlights IIUC.

More importantly using latest neovim (master) the ft_highlight feature is not working at all (I guess this might be related to them upstreaming some of treesitter functions?)

petobens avatar Aug 27 '22 19:08 petobens

That would probably defeat the purpose of having ft_highlights IIUC.

Yep that's my point. I think you'd accomplish what you're after by effectively using the highlights option. I'm going to park this issue for now. I'll leave it as an enhancement and time permits in the next month, I will take a look.

As always, would welcome a pull request for this.

olimorris avatar Aug 27 '22 19:08 olimorris

I'm going to park this issue for now

Ok!

More importantly using latest neovim (master) the ft_highlight feature is not working at all (I guess this might be related to them upstreaming some of treesitter functions?)

Can you check this one out though?

petobens avatar Aug 27 '22 19:08 petobens

Can you raise a new issue, specifically with the error messages and the version you're using? I've just pulled in a new nightly build and everything is okay.

olimorris avatar Aug 27 '22 20:08 olimorris

Closing this as per 56b3a7a75acc50b07e76d52e2ca678ada209cb20

olimorris avatar Oct 05 '22 11:10 olimorris