lukas-reineke / indent-blankline.nvim

Indent guides for Neovim
MIT License
4.07k stars 102 forks source link

feat: repeat indent guides on wrapped lines with 'breakindent' #810

Closed luukvbaal closed 8 months ago

luukvbaal commented 8 months ago

Opt-out with ibl.config.indent.repeat_linebreak = false.

Close #577

luukvbaal commented 8 months ago

Depends on https://github.com/neovim/neovim/pull/26625. Currently testing if it is included with pcall(nvim_buf_set_extmark, ...), may be replaced with a version check at some point I suppose.

One caveat here is that since extmarks are local to a buffer, and 'breakindent' is a window option, the indent lines may overlap buffer text when there are multiple windows with different 'breakindent' values displaying the same buffer... Doubt that is a real issue though.

lukas-reineke commented 8 months ago

Thanks, this is nice.

may be replaced with a version check at some point I suppose.

Yeah, version check would be cleaner. You can just add the version check now with 0.10. I only support latest nightly and stable.

One caveat here is that since extmarks are local to a buffer, and 'breakindent' is a window option, the indent lines may overlap buffer text when there are multiple windows with different 'breakindent' values displaying the same buffer... Doubt that is a real issue though.

This is fine, there are other option values with the same restriction already. I have this helper function to get the right window ID to use with nvim_get_option_value. Please use that as well. https://github.com/lukas-reineke/indent-blankline.nvim/blob/0dca9284bce128e60da18693d92999968d6cb523/lua/ibl/utils.lua#L131-L142

And please add the option to the docs as well.

luukvbaal commented 8 months ago

Hmm there is another caveat. Depending on the value of 'breakindentopt', we may still end up placing the indent guide virtual text over the actual buffer text. Not sure how to handle that TBH, here or in the nvim API.

Maybe we have to parse 'breakindentopt' as well, and only repeat if we can be sure the wrapped line is indented to the same column.

lukas-reineke commented 8 months ago

I will think about how to handle breakindentopt correctly. It's pretty niche, I'm okay with just not supporting this for now.

luukvbaal commented 8 months ago

I will think about how to handle breakindentopt correctly. It's pretty niche, I'm okay with just not supporting this for now.

That's my feeling as well. I'll address the comments/CI failure later.

mehalter commented 8 months ago

Should this be on by default? It seems like having the default options require the latest Neovim nightly is a bad idea even if the user can opt out of the new feature. It seems like it should be an opt in

luukvbaal commented 8 months ago

Whether an option requires a certain Neovim version seems irrelevant to the option's default value. I think most people will want this option enabled, that's why I suggested opt out.

I do agree that just checking for has("neovim-0.10") is more disruptive than it has to be for for people on a nightly version, but not the latest nightly. But they are using a nightly version, they can just update which I suppose was @lukas-reineke's view :)

mehalter commented 8 months ago

Yeah I agree. I'll be honest, I just saw in the docs that the default is true and I didn't notice the check for 0.10. this is a completely sane default and relying on people using nightly to just update makes sense for me! People using nightly accept the chance for bugs and potentially infinite amounts of instability 😂

lukas-reineke commented 8 months ago

The option will simply be ignored if you are on stable. And if you are on nightly, it's your responsibility to be up to date.