Skip to content

Conversation

@abeldekat
Copy link

See this discussion in LazyVim

The approach discussed has been merged in LazyVim and AstroNvim

TLDR:
Startup time performance is affected quite significantly when the clipboard provider is xsel(linux) or pbcopy(macos). I expect an improvement in these cases, especially on older pc's.

This PR makes sure that vim.opt.clipboard is only applied after UiEnter

@siduck, in case you are interested, please let me know if the code needs adjustments.
I "decorated" the config function defined in `Nvchad/starter".

@siduck
Copy link
Member

siduck commented Jul 24, 2024

@abeldekat
Copy link
Author

abeldekat commented Jul 24, 2024

https://github.com/NvChad/NvChad/blob/v2.5/lua/nvchad/options.lua#L12

you should add it here

Unfortunately, if the user decides to change the clipboard option in his own lua/options.lua, there is no way to reset and defer the option.

-- User options.lua in "NvChad/starter":

require "nvchad.options"
-- add yours here!

local o = vim.o
-- o.cursorlineopt ='both' -- to enable cursorline!

-- Not deferred!
o.clipboard = "unnamed" -- or a custom clipboard tool config, see ":h g:clipboard"

-- Nvchad options.lua in "NvChad/NvChad":

vim.schedule(function()
  o.clipboard = "unnamedplus" -- overrides user's changes...
end)

I also tried the approach in an autocommand on LazyLoad, added as the first statement to lua/nvchad/plugins/init.lua. That did not improve the performance, probably kicked in too late.

EDIT: An example of "doing something" with spec.config supplied by the user can be found here

@siduck
Copy link
Member

siduck commented Jul 24, 2024

ig its better to remove the cliboard function from main repo itself and let the user add it in his starter..

I dont get why such simple option should be heavy, i hope its not cuz of windows 😨 😡

@abeldekat
Copy link
Author

ig its better to remove the cliboard function from main repo itself and let the user add it in his starter..

Yes, that would be the best option. I think that existing users would not profit though.

I dont get why such simple option should be heavy, i hope its not cuz of windows 😨 😡

That's discussed here. It can happen on linux("xsel"), macos("pbcopy") and windows-wsl2, as far as I can see.

Would you like me to propose a PR on the starter?

@siduck
Copy link
Member

siduck commented Jul 24, 2024

what if the main nvchad's repo's option file had just this?

vim.schedule(function()
  o.clipboard = "unnamedplus" -- overrides user's changes...
end)

@abeldekat
Copy link
Author

That would work except when the user adds his own clipboard setting to lua/options.lua. The user could decide to opt-out (""), or change ("unnamed" for example) or provide a completely custom config according to :h g:clipboard. As the default vim.o.clipboard option also has the empty string as value, it is not trivial to distinguish between user-changes and that default.

The problem, I think, is that in the starter, in NvChad's config function, lua/options.lua is required. The effect is that the loading of the options is "in control" by the user, not by NvChad.

That's why I ended up with decorating that config function, taking back control. I think that works in 99.99% of the cases. However, because the user is free to put any code he desires inside the config function, the "timing" could still break. Highly unlikely in my opinion.

@abeldekat
Copy link
Author

@siduck,

I found a better approach and pushed a new commit. I used the local lua/options.lua to test:

  1. No override
  2. o.clipboard = ""
  3. `o.clipboard = "unnamed"
  4. vim.opt.clipboard = ""

The code now uses an autocommand on LazyDone: "when lazy has finished starting up and loaded your config"

@siduck
Copy link
Member

siduck commented Jul 25, 2024

@abeldekat the user can add his own vim.schedule... in his options file

@abeldekat
Copy link
Author

abeldekat commented Jul 25, 2024

Yes, the user can do:

o.clipboard = "" -- reset
vim.schedule(function()
  o.clipboard = "unnamedplus"
end)

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.

2 participants