SmiteshP / nvim-navic

Simple winbar/statusline plugin that shows your current code context
Apache License 2.0
1.35k stars 49 forks source link

Feat/lazy update context #120

Closed rasulomaroff closed 1 year ago

rasulomaroff commented 1 year ago

This covers the case where I always want to have updates on the CursorMoved event turned off. So, instead of setting vim.b.navic_lazy_update_context on every buffer and still firing the autocmds for that (BufEnter + CursorMoved), which is inefficient, I added the lazy_update_context property to the setup function that allows to turn off updates on the CursorMoved event by not setting the autocmd at all.

rasulomaroff commented 1 year ago

Well, do you think adding info on vim.opt.updatetime (which CursorHold event relies on) is necessary?

SmiteshP commented 1 year ago

I don't think this feature is necessary because as you said this can be achieved using the existing buffer variable as well. Additionally the buffer variable gives more flexibility to users for disabling updates based on some metrics (file size, language, etc). No need to add an redundant option to avoid a tiny if else check. What do you think?

rasulomaroff commented 1 year ago

I don't think this feature is necessary because as you said this can be achieved using the existing buffer variable as well.

Yes, it can, but then again, I turned it off for every buffer. So that means that I have 2 redundant autocmds. First - BufEnter, since I have to set that variable for every buffer. Second - CursorMoved, it will be triggered for every cursor move and that's totally redundant since I don't want that to happen for every buffer, not for specific ones.

No need to add an redundant option to avoid a tiny if else check.

This tiny if else check will only happen once - on attach. So, instead of adding tiny if checks on every cursor move (in my case, where I enabled lazy_context_update) and firing 2 autocmds BufEnter + CursorMoved, this config option will help to remove that redundancy.

SmiteshP commented 1 year ago

Hmm.. makes sense. Just one change request. In documentation mention how this option differs from the existing bug variable. Just so that novice users aren't confused about two different methods of disabling updates existing

rasulomaroff commented 1 year ago

Done! Is it okay now or there are other aspects that should be changed to be more clear?