lukas-reineke / indent-blankline.nvim

Indent guides for Neovim
MIT License
4.28k stars 106 forks source link

Highlight current indent (no color) with a lower priority than scope (with color) #649

Open Danielkonge opened 1 year ago

Danielkonge commented 1 year ago

Problem

I just updated to version 3 today and haven't yet played around with all the options, but I couldn't find something doing what I am asking for in the following. If it already exists, could someone let me know how to do it?

I really like the new highlighting via scope (especially in Rust). Even in Lua, I think this is helpful:

Screenshot 2023-09-28 at 23 34 33

So I plan to keep the highlighting on (I am matching it with rainbow-delimiters).

But whenever there is no scope highlighting I would like to fall back to slight highlighting of my current indentation level. I.e., in this picture:

Screenshot 2023-09-28 at 23 34 56

I would like to have some slight highlighting of the second indent line to the left of my cursor.

Preferably I would probably want to always have a slight highlighting of the current indent level unless that highlighting is overwritten by the scope highlighting. So sometimes the might be two highlighted indent lines at the same time. E.g., here:

Screenshot 2023-09-28 at 23 46 15

I would like to keep the first indent red, but have the second indent slightly light up to indicate the current indent level at the cursor position.

Is there already a way to do this? If not, I am willing to spend some time to try helping with implementation or testing of something like that. And if nothing else, I would like to request a feature like that.

Expected behavior

Always slightly highlight (e.g., just brighter color) the current indent level unless the current indent level is also the scope level in which case the scope is preferred for highlighting.

(Alternatively, when not using the current scope to highlight anything, highlight slightly the current indent level.)

lukas-reineke commented 1 year ago

No, it doesn't exist. There is a similar discussion in #632 to give some context. In short, this would be difficult to implement and maintain to the level of quality I want. I don't think I will work on this for now.

Of course if someone makes a great PR I am open to add this. If anyone wants to try, please discuss the implementation with me first, so you don't waste time on a suboptimal solution.

Danielkonge commented 1 year ago

I am willing to try to implement this. How would you want something like this to be implemented?

I haven't used priority in my own setup, so I am not sure if it works exactly as I would expect, but I thought we would be able to use that to prioritize the highlighting of the current indentation level lower than the scope highlighting. Is that correct?

So I assume the main work is to get the highlighting of the current indentation level to work. I haven't looked much at the code yet, so I am sure I will find out that it is much harder than it looks quickly, but I thought we would have access to the indent lines drawn on the current cursor line and could relatively simply "catch" the inner most one and highlight that level of indentation. I assume some of the complexity comes from how the indentation blocks are made, but am I missing anything else?

Also, when I work on this, which files would I need to edit? (Where would you prefer this code to be?)

lukas-reineke commented 1 year ago

I thought we would have access to the indent lines drawn on the current cursor line and could relatively simply "catch" the inner most one and highlight that level of indentation.

Yeah it's not that easy. The code iterates only a single time over the lines for performance. So when you reach the cursor, all lines above are already rendered. You can't change the highlight anymore.

The scope works by walking the treesitter tree up from the cursor before looping over the lines, that is very fast. Any solution for a "current" indent would need to do something like that.

I added wildcards for the scope in version 3.2.0. That gets pretty close to "current" indent, but it has a lot of edge cases.

Danielkonge commented 1 year ago

I have looked at the code now and can see that it will be a bit more complicated than I thought. Would something like this work:

(I know this is not really mentioning any exact implementation details, but I mostly just want to get a general picture to start with.)

lukas-reineke commented 1 year ago

yeah these details don't really matter. The main question is just how the algorithm should work.

Danielkonge commented 1 year ago

I think what I want can almost be done with small changes in the scope code. I have uploaded a very rough draft here: #668

It is not at all finished yet, but I do get the behavior I want in most cases I have checked now.

I plan to work more on this tomorrow and maybe the day after, but I thought I would upload it for now, if you had any early comments about it.

Danielkonge commented 1 year ago

Here are some comparison pictures.

When the scope is the inner context, we only highlight the scope:

Screenshot 2023-10-01 at 01 42 12

When there is no scope highlighting, we highlight the context:

Screenshot 2023-10-01 at 01 41 19

When the scope is further out than the context, we highlight both:

Screenshot 2023-10-01 at 01 40 32

And again with the scope being the same as the context:

Screenshot 2023-10-01 at 01 40 10

But it will (mistakenly) highlight what is a your current indentation level, even if it is not in the same context:

Screenshot 2023-10-01 at 01 49 30

Though it will be correct as soon as you go into the context:

Screenshot 2023-10-01 at 01 49 45 Screenshot 2023-10-01 at 01 50 00

I don't actually really mind the behavior above, but I don't think it is what most people expect/want.

Maybe if you are fine with it, we could keep a behavior like that and call it something like indent_level to avoid confusion with context.

lukas-reineke commented 1 year ago

call it something like indent_level to avoid confusion with context.

Yes, name should definitely be something like current_indent

Danielkonge commented 1 year ago

I have renamed it to current_indent and rewritten a little bit now.

The algorithm is still almost an exact copy of scope that just basically counts almost everything as being a "scope". And then you have to for each language figure out how that language's tree sitter implementation represents blank lines and lists of commands, so that you can make sure not to highlight by mistake as above in these cases.

I think a lot of this could be moved into the implementation of scope if you would prefer that, but it makes sense to have these two almost identical fields, since I don't think you can use scope for multiple different things at once in a similar way without it?

I have tested on the following file types now:

And most things works as I would want/expect.

I think one problematic thing about the way I am doing it now is that you have to maintain a file like: https://github.com/lukas-reineke/indent-blankline.nvim/blob/master/lua/ibl/scope_languages.lua

And from what I can tell that file was generated, while I don't see anything in neovim's tree sitter implementation that I could use to generate what I use in current_indent.

I haven't run into many extra edge cases after considering the "blank line" types mentioned above, but I have seen a few and I also haven't tested that much.

It is mostly pretty easy to debug. You just have to do something like :lua print( require("nvim-treesitter.ts_utils").get_node_at_cursor():type() ) and/or the tree sitter playground, where you think the highlighting is not as you want, and then you can edit the setup accordingly. But some things can't be easily fixed with what is available in tree sitter.

I agree that this is not at a level, where you would want to accept the draft pull request, but since it works mostly as I wanted now, I don't think I will do much more work on it unless I discover a much better way or you want me to change anything specific and work towards a proper pull request?

I will try to look more at tree sitter, but it doesn't seem to expose the things I would like to use to improve what I have for now.

(Also, I can close/delete the draft pull request if you prefer?)

lukas-reineke commented 1 year ago

I think one problematic thing about the way I am doing it now is that you have to maintain a file like

Yeah this is a dealbreaker. Maintenance effort would be way too high. And this would never be perfect. I think treesitter is fundamentally not the right approach. It is perfect for scope, but treesitter has no concept of the current indentation. I can format my code differently, and it all falls apart.

simonhughxyz commented 1 year ago

I would also like to see indentation level "context" be highlighted, I was not even aware that there was a problem with the old current context, it always worked fine for me.

Instead of using treesitter, why not just use old fashioned regex? Why does it have to be treesitter? Could you not just look at the indent of your current line, and then look above and below your current line till you meet a line that does not match the indentation of your current line?

lukas-reineke commented 1 year ago

Could you not just look at the indent of your current line, and then look above and below your current line till you meet a line that does not match the indentation of your current line?

It has to be something like this, yes. But doing it separate from the main loop would kill performance. The only real solution I see it to split the calculation and rendering into two loops. Then the calc loop can walk back to update previous lines. But this adds a lot of complexity. I am not convinced it is worth it.

Danielkonge commented 1 year ago

I had some time today too, so I tried making a version without tree sitter where most of the code is completely separate from the tree sitter stuff, so if you disable `current_indent´ you shouldn't notice any difference in speed although this is slower than tree sitter: https://github.com/lukas-reineke/indent-blankline.nvim/pull/679 (I have a lot of free time this week...)

On my machine it works fine, but I don't know if that is true for most people. It is again not ready for pull request yet, but I just thought I would ask if you would be more willing to accept something like that? I am pretty sure there is a slow down when enabling it, but when it is disabled, everything should feel the same, and then it is up to the individual user, if they want to use it. I haven't found any edge cases so far, and I think it should basically always work (can't see what could go wrong if the indents are drawn correctly already).

lukas-reineke commented 1 year ago

I commented on the PR

I had a look at the code again. Let me explain what I think is the only reliable solution for this.

  1. When calculating the whitespace, you have to calculate into the future until you find the indent of the cursor, cache all the results, and then pop them off the stack on the next iterations. Similar to how blanklines work.

https://github.com/lukas-reineke/indent-blankline.nvim/blob/906ba1e843063d29467663d53a86014f0ece1bb9/lua/ibl/init.lua#L283-L316

Basically, next_whitespace_tbl would need to become an array of whitespace_tbl.

  1. Once you know the cursor indent, it should be easy to duplicate the logic of scope

But this is very complicated. It has to work with all the other logic that is already there. Even if you make a perfect PR I can't promise I'll merge it. If it is too complex, I might decide that the maintenance effort is not worth it.

Danielkonge commented 1 year ago

I looked into doing what you said, but it was hard to get it to work with everything else.

What I ended up doing was split the for loop into two loops.

The first loop goes over each line and makes an array of whitespace_tbls and a corresponding array of each table's (i.e., each line's) furthest indent.

Then after the first loop I use the array of each line's furthest indent to calculate the current row's indent and find the start and end of the current indent level (this is easy and quick since I have an array with just numbers to look at for the indent levels).

Then the second loop goes over each line and makes the virtual text and sets up extmarks using the array of whitespace_tbls.

Realistically keeping the array of whitespace tables is not optimal, but I don't notice any slowdown when using this setup on my computer and I have everything working as I wanted now.

As mentioned in the previous pull request, I don't really expect this to be committed here, but I wanted something like this for my own editing and I am also just trying to get used to writing more complicated Lua code. I will probably just keep my fork that is slightly slower (I think, it doesn't feel slower), but does what I want for now, but if I figure out a smarter way to do this I will probably ask if you will accept a pull request.

(I don't know a good way to compare code without making a pull request, but here is the new init.lua (where most of the changes are) that I am using myself: https://github.com/Danielkonge/indent-blankline.nvim/blob/no-treesitter/lua/ibl/init.lua )

By the way, while I kept looking at this and asking about it, I just want to reiterate that I think the scope highlighting of v3 of ibl is really good! So even if you never add a feature like this yourself, I think it is worth the upgrade.

lukas-reineke commented 1 year ago

It's definitely the best attempt so far. But the 2 loop solution is not ideal. You now do duplicate work in both.

Danielkonge commented 1 year ago

I changed it slightly by adding a boolean array keeping track of which is are skipped (since they already do what they need to in the first loop), but I don't think you can get around doing a little bit of extra work by doing two loops (unless you add even more arrays, but that doesn't seem like a good idea either, except for some computationally heavy stuff).

Danielkonge commented 1 year ago

Actually, what I said is not entirely correct. With only adding a few arrays containing either booleans or numbers (i.e., arrays that should have no influence on the performance when we work with less than a thousand lines and do other more calculation heavy stuff), I got it so that I don't do any calculation twice (other than an integer addition since it seemed like overkill to save that in an array...).

I think now the only thing that might impact performance is keeping around the array of whitespace_tbls, since you could have a large number of indents. Though that array should realistically almost always be under 100 kB in size (you make the whitespace_tbl with numbers of 8 bytes each and most files will have an average indent of less than 50 spaces and need less than 200 lines highlighted).

lukas-reineke commented 1 year ago

Yeah, keeping the whitespace_tbl is fine.

You can improve the calculation for the current indent by having a stack that mirrors indent_state.stack, but with the line numbers. If you add a new indent to the indent_state stack, then you add the line number to the current_indent stack. You pop one of, you pop one of from the other as well. That way, when you reach the cursor, you can just take the line number that's on top of the stack and that's your start line. And then finding the end is easy. This way, you don't have to do any extra looping for the current indent. (it probably won't make a big difference, but might as well)

Overall, this solution is probably as good as it will get. Please make a PR. Before I merge it, I'll test it properly and might refactor it a bit, but not sure when I have time for that.

Thanks for being persistent, I think there is a good chance we can get this merged.

chaneyzorn commented 1 year ago

Hi, I've used both indent-blankline.nvim and mini.indent at same time. And so far everything looks fine.

Look at following screenshot :

image

Just sharing, also expect the discuss of here.

Danielkonge commented 1 year ago

You can improve the calculation for the current indent by having a stack that mirrors indent_state.stack, but with the line numbers. If you add a new indent to the indent_state stack, then you add the line number to the current_indent stack. You pop one of, you pop one of from the other as well. That way, when you reach the cursor, you can just take the line number that's on top of the stack and that's your start line. And then finding the end is easy. This way, you don't have to do any extra looping for the current indent. (it probably won't make a big difference, but might as well)

I have added a stack now, but I am not sure if you meant to add it in the indent_state? (That's not what I did.)

Overall, this solution is probably as good as it will get. Please make a PR. Before I merge it, I'll test it properly and might refactor it a bit, but not sure when I have time for that.

Thanks for being persistent, I think there is a good chance we can get this merged.

I have made a pull request now. If there is anything specific you would like me to edit, you can just let me know, and I can do some stuff at first before you go over it yourself.

ZeChArtiahSaher commented 1 year ago

guys could you update some kind of doc or post your setups? I can't literally even wrap my head around getting to highlight indent block with the config because neither the help section or readme is helpful

Danielkonge commented 1 year ago

guys could you update some kind of doc or post your setups? I can't literally even wrap my head around getting to highlight indent block with the config because neither the help section or readme is helpful

The current_indent implementation hasn't been merged yet. The current work has been moved to #743 and I think it will be ready to be merged soon. When it is merged, part of the update includes docs about setting up current_indent.

ZeChArtiahSaher commented 1 year ago

And also like in general I think there isn't even an updated example of how to setup highlights either

PerchunPak commented 8 months ago

Really sad this feature was abandoned.

However, I managed to do something similar using both this plugin and mini.indent

image

But the downside of this approach is that mini.indent overwrites indent-blankline, and you can see such a situation: image (line is gray, which means mini.indent is on top of indent-blankline)

Config And because I couldn't found how [chaneyzorn](https://github.com/lukas-reineke/indent-blankline.nvim/issues/649#issuecomment-1748443269) did this (thanks for the idea btw!), here is my config: ```lua require('mini.indentscope').setup({ ["draw"] = { ["animation"] = function() return 0 end, # disable animation ["delay"] = 0, ["priority"] = 2 }, ["options"] = { ["border"] = "top", ["try_as_border"] = true }, ["symbol"] = "▎" }) require('ibl').setup({ ["indent"] = { ["priority"] = 1, ["smart_indent_cap"] = false }, ["scope"] = { ["show_end"] = false, ["show_start"] = false } }) ``` P.S. These lines were generated by [NixVim](https://github.com/nix-community/nixvim), I just formatted them nicely. There might be other configuration lines that I didn't notice (tip for those nixvim users: `animation = with inputs.nixvim.lib.x86_64-linux.helpers; mkRaw "function() return 0 end";`).
evertonse commented 4 months ago

Excited to see this <3, I'll drop mini.animate as soon as this is out