Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(winbar): don't modify non-heirline winbar for disabled buffers #166

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

mehalter
Copy link
Contributor

@mehalter mehalter commented Aug 29, 2023

Rather than setting disabled buffers to nil it would be better to just leave the winbar untouched if it was not previously set by Heirline. This is much better for buffers that want to handle their own winbar entirely such as neo-tree which can lead to some major flickering.

Let me know what you think or maybe if we want to add a separate option for this case like disable_winbar_cb and ignore_winbar_cb, but I thought that might end up being a bit confusing to the user.

Example flickering:

Screen Recording 2023-08-22 at 12 20 06 PM

@mehalter mehalter force-pushed the master branch 4 times, most recently from d7718d8 to b482f1c Compare August 29, 2023 19:39
@mehalter mehalter changed the title fix(winbar): don't modify winbar for disabled buffers fix(winbar): don't modify non-heirline winbar for disabled buffers Aug 29, 2023
Copy link
Owner

@rebelot rebelot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that sounds sensible, it should also be tested launching nvim with file arguments

lua/heirline/init.lua Outdated Show resolved Hide resolved
@rebelot
Copy link
Owner

rebelot commented Sep 3, 2023

btw, I realised I encountered this problem with Neo-tree and I used this work around

        disable_winbar_cb = function(args)
            if vim.bo[args.buf].filetype == "neo-tree" then
                return
            end
            return conditions.buffer_matches({
                buftype = { "nofile", "prompt", "help", "quickfix" },
                filetype = { "^git.*", "fugitive", "Trouble", "dashboard" },
            }, args.buf)
        end,

@mehalter
Copy link
Contributor Author

mehalter commented Sep 5, 2023

@rebelot I update my commit to fix that or -> and. Is that all that you think needs to change?

it should also be tested launching nvim with file arguments

Is this being taken care of in that autocmd? Or do I need to add the check somewhere else? Thanks for your help on this!

@rebelot
Copy link
Owner

rebelot commented Sep 5, 2023

Great, that's all. The autocmd should take care of startup, I just wanted to make sure that this PR does not screw up winbars when launching nvim with multiple arguments (usually diff mode, if not taken care of, only the first buffer will have the winbar set due to some autocmd hell)

@mehalter
Copy link
Contributor Author

mehalter commented Sep 8, 2023

@rebelot have you had a chance to test this out with all of the arguments you wanted to? :)

@mehalter
Copy link
Contributor Author

mehalter commented Oct 2, 2023

Hey @rebelot just wanted to do a quick bump on this to see if you've had a chance to test it out?

@mehalter
Copy link
Contributor Author

Hey @rebelot, I'm sorry to be an annoyance and keep pinging. Is there any chance this is able to get merged yet?

@mehalter
Copy link
Contributor Author

I have tested this with many different starting arguments and situations (opening multiple files at once, opening several files in splits, etc.) and it doesn't appear to have any issues at all.

@rebelot rebelot merged commit 170e1b1 into rebelot:master Nov 29, 2023
1 check passed
@rebelot
Copy link
Owner

rebelot commented Nov 29, 2023

Hi @mehalter , I am very sorry for the delay in getting back to this!

@mehalter
Copy link
Contributor Author

No worries at all! I appreciate all your work on this project :) no need to apologize for taking some time away, open source maintenance can be taxing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants