-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: Statusline caching and custom functions #155
Conversation
Hey @abeldekat! Thanks for taking the time to improve statusline integration for Grapple. So, I decided to go ahead and do some very basic benchmarking here. Calling Without caching
With caching
While the cached result is faster, the uncached result is still negligible compared to the polling rate of
I do feel that lualine can lack in "responsiveness" sometimes, which I liked that you pointed out. Would this be resolved with the following autocommand (something a user could add to their config)? vim.api.nvim_create_autocmd("User", {
pattern = { "GrappleUpdate", "GrappleScopeChanged" },
callback = vim.schedule_wrap(function()
require("lualine").refresh()
end,
}) |
You're welcome! I had a lot of fun working on this PR...)
I currently work on a slow laptop(intel i3, 4G RAM). I regularly see "hesitations" when working with I think the benchmarking results are impressive.
Frankly, I don't know. I could not get the new component to work with |
I wouldn't mind digging into this a bit. Would you mind clarifying what you mean by "hesitations"? Do you find neovim slowing down when you use lualine and grapple? Is the slowness fixed by removing one (or both) of them? Or, do you just mean that lualine doesn't always immediately reflect the current state of grapple (i.e. add a tag, but it doesn't show up right away)?
I have an idea of what the culprit might be here: lua/grapple.lua#L675. Grapple watches for
However, I don't necessarily think this is Grapple's fault. Notice, if you replace (in the above autocommand) |
The latter indeed. I should have been more clear by using the word "responsiveness".
The error occurs on startup when first using stacktraceError executing Lua callback: ...nvim/site/pack/deps/opt/grapple.nvim/lua/grapple/tag.lua:36: BufEnter Autocommands for "*"..User Autocommands for "GrappleUpdate"..User Autoc
ommands for "GrappleUpdate"..User Autocommands for "GrappleUpdate"..User Autocommands for "GrappleUpdate"..User Autocommands for "GrappleUpdate"..User Autocommands for "Grapp
leUpdate"..User Autocommands for "GrappleUpdate"..User Autocommands for "GrappleUpdate"..User Autocommands for "GrappleUpdate": Vim(append):E218: Autocommand nesting too deep
stack traceback:
[C]: in function 'command'
...nvim/site/pack/deps/opt/grapple.nvim/lua/grapple/tag.lua:36: in function 'select'
...are/nvim/site/pack/deps/opt/grapple.nvim/lua/grapple.lua:147: in function 'callback'
...e/pack/deps/opt/grapple.nvim/lua/grapple/tag_manager.lua:54: in function 'enter'
...nvim/site/pack/deps/opt/grapple.nvim/lua/grapple/app.lua:189: in function 'enter_without_save'
...are/nvim/site/pack/deps/opt/grapple.nvim/lua/grapple.lua:135: in function <...are/nvim/site/pack/deps/opt/grapple.nvim/lua/grapple.lua:129>
...are/nvim/site/pack/deps/opt/grapple.nvim/lua/grapple.lua:678: in function <...are/nvim/site/pack/deps/opt/grapple.nvim/lua/grapple.lua:650>
I don't think so as well. I do think that decorating grapple's api to push events works really well. Might even be a bit faster as it does not involve Neovim's autocommand machinery. |
It works when using function Statusline:subscribe_to_events()
vim.api.nvim_create_autocmd({ "BufEnter" }, {
group = STATUSLINE_GROUP,
pattern = "*",
callback = function()
self:update()
end,
})
vim.api.nvim_create_autocmd({ "User" }, {
group = STATUSLINE_GROUP,
pattern = "GrappleScopeChanged",
callback = function()
self:update()
end,
})
vim.api.nvim_create_autocmd({ "User" }, {
group = STATUSLINE_GROUP,
nested = false,
pattern = "GrappleUpdate",
callback = function()
vim.schedule_wrap(function()
self:update()
end)
end,
})
end
|
Why is the |
When changing buffers the statusline needs to be updated: State 1: |
Okay, so the issue is here.
|
Why? I don't expect select or find to create an update event. Update: I would expect the event to happen when calling the api methods I decorated in |
The purpose was intended that any interaction with Grapple (tag, untag, select) would cause an update (see: lua/grapple/tag_manager.lua#L43-L73). It was probably an oversight to have it trigger for API calls which only query Grapple (find, exist, name_or_index). |
That explains. What is your opinion on the current approach, using method |
I added one commit, improving the custom formatter example in the tests |
Hi @abeldekat, apologies for the delay here. Was unable to continue reviewing the other week. I had a chance to investigate the event issue. I don't think that the decoration approach is quite the solution. I took a bit of time to refactor how the Grapple API and App work to make it so that the events now trigger in the right place. You can see the PR here: #158 (It's 95% moving code around). I also took another look over this PR. After some thought, I don't think this is something that I want Grapple to maintain in the long run. This is primarily due to a few things:
For those reasons, I don't think I'll be able to merge this PR. However, I would be happy to add grappleline to the README for users who want that kind of flexibility with lualine as I do think there's some great value add, especially for those using lualine (and mini.statusline) which does not natively support autocommand-driven updates. |
Hello @cbochs, I understand your choice. The I added the code to my own config and intend to remove the plugin. Best regards! |
This PR implements issue feature: Statusline caching and custom functions #147 and adds the following:
the caching and the data provided by
grapple
.Changes
grapple.statusline
and corresponding tests.grapple.settings.statusline
.Grapple.statusline
: Uses the new component. Does not returnnil
anymore.lualine grapple
component: Changed to use the new version ofGrapple.statusline
.The second
lualine
example in the docs uses two api methods which are not changed:The same can now be achieved by using:
Considerations
The statusline is opt-in and only activates when the user invokes
Grapple.statusline
. As such it is a top level component.Optimize the number of steps in the code needed to produce a line after the initial setup. As a consequence, method
Grapple.statusline
has noopts
argument that would need to be merged.Ultimately, only 2 steps are needed:
require("grapple.statusline").get()
line:format()
Works across statusline plugins in the same manner. Currently, the second example in the docs is
lualine
specific.Autodetect the statusline plugin in use and provide an
on_event
callback. This considerably improves the responsiveness oflualine
.Suggestions
Nice to have perhaps: Extend the default formatter by adding empty_slots, more_marks and scope_name. See test case "custom formatter" and the
grappleline
plugin.Remove the second
lualine
example from the docs, in favor ofbuiltin_formatter = "short"
.Questions
Api
The new statusline component has a public api:
What's a good way to distinguish that api from all the other methods in the component?
Events
When using event
GrappleUpdate
, the dreadednested
error occurred:Perhaps it's better to not use the events provided by grapple. In my opinion, the
api decoration
is a good solution.See also
Using mini.statusline in my config: config-mini.statusline config-grapple deps-editor deps-ui
Todo
Update the documentation.