-
-
Notifications
You must be signed in to change notification settings - Fork 109
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: current indent #743
base: master
Are you sure you want to change the base?
feat: current indent #743
Conversation
Co-authored-by: Lukas Reineke <lukas.reineke@protonmail.com> Co-authored-by: Daniel Kongsgaard <daniel@Daniels-MBP.home>
70bbdf3
to
ae26f0c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have written some comments now. I will try to use this a bit and go through the code more thoroughly to see if notice anything else.
yeah... no this doesn't work I'll try to rewrite it again next week |
Just a clarifying note: I noticed that it is actually going multiple indents in at once that is the problem. Not going multiple indents back. |
@@ -46,6 +49,9 @@ local setup_builtin_hl_groups = function() | |||
if not_set(get(ibl_scope_hl_name)) then | |||
vim.api.nvim_set_hl(0, ibl_scope_hl_name, line_nr_hl) | |||
end | |||
if not_set(get(ibl_current_indent_hl_name)) then | |||
vim.api.nvim_set_hl(0, ibl_current_indent_hl_name, line_nr_hl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the highlights from "CursorLineNr"
(or maybe just white) makes more sense here as the default highlight for current_indent
. I know it might not be set for everyone, but we could use this as a backup then instead. This would (when it is set) make it easier to distinguish between scope
and current_indent
for people trying out both.
Alternatively, it might be a better idea to just add an example with "CursorLineNr" as the highlight group for current_indent
in the docs, since I feel like that might be a common highlight choice.
if indent_state.stack[j].indent < current_indent_col_start_single then | ||
break | ||
end | ||
current_indent.start_row = indent_state.stack[j].row + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if indent_state.stack[j].indent < current_indent_col_start_single then | |
break | |
end | |
current_indent.start_row = indent_state.stack[j].row + 1 | |
current_indent.start_row = indent_state.stack[j].row + 1 | |
if indent_state.stack[j].indent <= current_indent_col_start_single then | |
break | |
end |
This partially fixes the problem with multiple indents for me (I will make another note to fix the other problem).
and config.scope.priority >= config.current_indent.priority | ||
) | ||
then | ||
vim.api.nvim_buf_set_extmark(bufnr, namespace, row - 1, current_indent_col_start_single, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vim.api.nvim_buf_set_extmark(bufnr, namespace, row - 1, current_indent_col_start_single, { | |
vim.api.nvim_buf_set_extmark(bufnr, namespace, row - 1, #whitespace, { |
We need to start highlights after the whitespace here (any highlights before the whitespace ends is already taken care of in the virtual text overlay). [Note: The code before the suggestion doesn't work with tabs since it depends on the whitespace_tbl
instead of whitespace
.]
Together with the above fix, this fixes the multiple indent level jumps problem for me (based on my testing so far).
The scope is *not* the current indentation level! | ||
See |ibl.config.current_indent| if you want to highlight the current | ||
indentation. | ||
Instead, scope it is the indentation level where variables or functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead, scope it is the indentation level where variables or functions | |
Instead, scope is the indentation level where variables or functions |
@@ -131,6 +155,8 @@ | |||
---@field whitespace ibl.config.full.whitespace: ibl.config.whitespace | |||
--- Configures the scope | |||
---@field scope ibl.config.full.scope: ig.config.scope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
---@field scope ibl.config.full.scope: ig.config.scope | |
---@field scope ibl.config.full.scope: ibl.config.scope |
Another small note: While doing a lot of undos in a larger file, I got the following error: Error executing lua callback: ...nal/nvim-plugins/indent-blankline.nvim/lua/ibl/utils.lua:264: attempt to perform arithmetic on a nil value [Update: I figured out a way to reproduce the problem, and it was not with undos. I have submitted a fix below.] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GitHub won't let me put this comment in the code for some reason, but there is a problem at line 328 of init.lua
.
if i == #lines then
whitespace_tbl = {}
we need to do something with the indent_state too, otherwise current_indent.show_start
will mistakenly highlight the line above the last line of the code (if the last line is empty).
if i == #lines then
whitespace_tbl, indent_state = indent.get(whitespace, indent_opts, indent_state, row)
works, but I am not sure if that is the best way to do it. Maybe it is better to set it explicitly. Will the last line ever be visible if it is not the last line of the file? If not, we could just set indent_state.stack = {{ indent = 0, row = row }}
in that if
statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a fix to a problem giving the following error.
Error executing lua callback: ...nal/nvim-plugins/indent-blankline.nvim/lua/ibl/utils.lua:264: attempt to perform arithmetic on a nil value
local end_pos_col | ||
|
||
if end_line[1] then | ||
end_pos = vim.inspect_pos(bufnr, end_row, end_line[1]:find "%S" - 1, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
end_pos = vim.inspect_pos(bufnr, end_row, end_line[1]:find "%S" - 1, { | |
local end_pos_find = #end_line[1] - 1 | |
local try_find = end_line[1]:find "%S" | |
if try_find then | |
end_pos_find = try_find - 1 | |
end | |
end_pos = vim.inspect_pos(bufnr, end_row, end_pos_find, { |
The current code causes problems when find returns nil
sometimes.
Just a quick update. I have used a fork of this with the fixes I describe above for a couple of weeks now. I have been editing files with both tabs and spaces for indentation, and I haven't noticed any other problems. |
@Danielkonge could you link the fork you have been using? I would be interested in giving it a try! |
It's this fork: https://github.com/Danielkonge/indent-blankline.nvim/tree/current_indent_final Note that this doesn't include any of the fixes that has been committed to the master branch in the last couple of weeks. This is mainly to test out Other than some comments the only real difference between the fork and this pull request is on lines: 328, 407-410 and 573 in |
No problem! I am just interested in investigating the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current_indent_enabled | ||
and config.current_indent.show_end | ||
and is_current_indent_end | ||
and #whitespace_tbl > current_indent_col_start_single |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and #whitespace_tbl > current_indent_col_start_single | |
and current_indent.start_row <= row | |
and #whitespace_tbl > current_indent_col_start_single |
until those PRs gets merged AstroNvim/AstroNvim#2365 lukas-reineke/indent-blankline.nvim#743
until those PRs gets merged AstroNvim/AstroNvim#2365 lukas-reineke/indent-blankline.nvim#743
Just a quick update, I am still planning on merging this. But it needs more polish, and I am very short on time at the moment. |
Thanks for the update, looking forward to this improvement! Appreciate all your work on this! |
This comment was marked as resolved.
This comment was marked as resolved.
@Danielkonge Sounds good, but please make it a separate PR. |
Hey! Just wanted to check in and see if this PR is still planned on getting merged at some point :) |
It is still planned. Unfortunately, my situation hasn't changed much. This year has been super busy, I just don't have that much time for open source. |
For whatever is worth, this would be an amazing feature to add. I love the notion of just having guideline essentially highlighted for the current indentation. It solves a lot of visual noise that can happen, specially around rainbow guidelines. Open source is pretty ungrateful, so regardless, this is an amazing plugin, and thank you for your hard work! |
@Danielkonge I finally had some proper time to go over your PR.
Like I mentioned before, I did some pretty extensive refactor. Your code was good, but there were things I wanted to do a bit differently.
current_indent_stack
, I reusedindent_state
show_end
for current indent to match scope betterCan you take a look at this? It's very likely I broke something that was working in your version.