fix: improve block and list indentations

[PriceHiller]

What does this do?

• Enables aligning blocks to the nearest listitem or headline
• No longer modifies the bodies of blocks beyond ensuring the minimum indentation in a block is aligned with the block header/footer
• Returns the "proper" indent level for a list item based on the header in indent mode instead of alignment purely to the list. This allows blocks to be properly aligned to new indent levels in listitems and better follows how the other category of indents works. This is done via recursive call to get the correct indent level up the tree of the listitems.

Additional Notes

I made this with the intention of rebasing #627 on it to resolve note 5 within #627. I found this to be the most obvious solution.

I extracted large sections of the indentexpr into a separate top level function in indent.lua so it can be used as part of the get_matches function. As such the diff is quite noisy. It may be easier to follow this PR via the commits than the mainline diff.

The logic for the blocks is almost entirely done in the get_matches function as I needed access to the treesitter nodes without the current indents being modified as part of the indentexpr. This ensures visual range indents work as expected. Doing the block indents as part of get_indent_for_match will cause issues in visual range as the indent can change for each line in the range.

I have written some tests for the new block functionality in the indent_spec.

@kristijanhusak If this gets merged I'll likely rebase #627 on top of this and use the work here as part of the finishing touches to #627.

[kristijanhusak]

Thanks for the PR.
Code looks ok, but I would need more time to fully understand it.

Can you give some examples that represents what this is fixing?

From the description, I tried applying indent to this:

* TODO Test list
  DEADLINE: <2023-11-22 Wed 13:43>
  - list item
    - nested list item
  #+begin_src lua
print('foo')
 #+end_src
    - bar
    - baz
* Other

and the result is this:

* TODO Test list
  DEADLINE: <2023-11-22 Wed 13:43>
  - list item
    - nested list item
  #+begin_src lua
  print('foo')
  #+end_src
  - bar
  - baz
* Other

Block got aligned to a list item which is somewhat expected,
but bar and baz got aligned to a list item where I would not expect it to happen.
Note that this is also broken on master, so it's not introduced here, just giving my observations.

I just need some examples to see what was fixed to better understand it. Thanks!

[PriceHiller]

So those are aligning to the header actually. I'm guessing you have indent mode enabled.

foo and baz got aligned to the headline TODO Test List.

In terms of "fixing" the block indents, it now ensures that the content inside the blocks aren't realigned and only indented to be in line with the block itself, no actual internal content is messed with beyond that.

In terms of "fixing" the list indents. It's more of a change than a fix I suppose.
Given some content like so and assuming indent mode is on:

* My Header
- My list
  - sublist

If I were to attempt to indent the sublist beforehand it would not actually change its indent level despite it technically being underindented. It was aligning to the parent. That's ok, but created issues for me in the promotion and demotion autoalignment in #627. Now if we attempt to indent only the sublist line, it gets set to the "correct" indentation level instead of binding only to the parent. It tries to stick under the parent while also knowing how deep it should be based on its nesting. So if I indented the sublist line with my changes it would look like so afterwards:

* My Header
- My list
    - sublist

This may seem wrong at first glance, but plays nicer with other sections of the code base as well as ensuring block indents align to the new list indents if a block is under the list item. Without that behavior it becomes much harder to know where to indent the individual blocks to if I don't invoke a visual range on the entire section.

Take for instance this video of before my changes (as a note every time I make a visual selection I am hitting my indent key =):

before-changes.mp4

And after:

after-changes.mp4

You'll notice that the primary issue in the before is that the content body doesn't get reindented. The reason why I went a bit further than that is so I could get the correct indentation as expected in #627 for the virtual indents especially on promotions and demotions.

The major thing this PR brings is alignment of misaligned block contents. If you have content in a block like so:

**** Headline
     #+begin_src json
{
   "key": "value"
}
     #+end_src

It will now align the underindented block contents to be aligned with the block after indent like so:

**** Headline
     #+begin_src json
     {
        "key": "value"
     }
     #+end_src

It also works with overindented internal block content:

**** Headline
     #+begin_src json
          {
              "key": "value"
          }
     #+end_src

Becomes:

**** Headline
     #+begin_src json
     {
         "key": "value"
     }
     #+end_src

Of significance is the approach to the indentexpr. I am now somewhat ignoring alignment to parent nodes, instead returning the indent of what the current line should be if the parent nodes were correctly indented.

[kristijanhusak]

Thanks for the detailed explanation. Videos are very helpful.

I think having a single indent function to handle both indent and non-indent configuration options is not the optimal solution.

What would be the complication if we create 2 separate functions that handle the indents separately? Even though I understand the reasoning behind the changes to the list item indentation, I do not really like the way it works. I would expect it to align to the parent list item, at least in an indent mode. Also regarding the blocks, I'm not sure if it's a good idea to dedent the content of the block to match the start of #+begin_src, since the content might be indented intentionally.

[PriceHiller]

When it came to matching the start of #+begin_src I quite literally can't think of a single reason to want additional padding within the block itself. I don't know of any reason to do so, but if you believe it shouldn't be dedented then I'm fine with yanking that, not a big deal. Just kind of a QOL thing I thought would be nice. I remember why I did this now. This was done such that when blocks got dedented (e.g. a headline) got demoted, the internal content was correctly dedented as well. Without this change, handling that scenario is much more difficult.

I don't think two entirely separate functions are needed seeing as you'd like keep the parent alignment. Some reorganization would be good though. What could be done is apply the indents relative to the parents and then apply padding at the very end of the indentexpr function from a separate indent-mode-padding function or something like it. Would simplify it quite a bit.

As a heads up the indents do still respect the parent nodes, just to a lesser degree. They're jumping to the intended indent if the parent were correctly indented, not the indent required to be in line with the current parent indent.

If you can give some hard answers in a "yes, do that" or "no, don't do that" format to the below, I'll gladly do a bit of refactoring to be more in line with what you'd desire.

1. Do you want me to fully respect the parent node? For example, don't attempt to indent to the "correct" position if the parent were correctly indented, indent to the current indent parent only. If yes, then I'll have to figure out a different solution of some sort over in Add Virtual Indent #627.
2. Do you want me to remove the dedent to match the blocks if the block content is overindented? Again, I can't think of a reason why you wouldn't want to be aligned to the block, but I'll do it if you prefer it this way. See modified comment at the top of this block.

[kristijanhusak]

When it came to matching the start of #+begin_src I quite literally can't think of a single reason to want additional padding within the block itself. I don't know of any reason to do so, but if you believe it shouldn't be dedented then I'm fine with yanking that, not a big deal. Just kind of a QOL thing I thought would be nice.

I thought it might not be dedented properly if the code itself has strange indenting, but I see it respects the least indented line and aligns everything properly, so I'm ok with leaving that.

I was thinking about 2 separate functions because it will be probably easier to maintain later. I'm expecting that we will get more issue reports once the virtual indents kick in, so it might be difficult to distinguish the code that needs fixing, and might be harder to debug.
If you think that'll be too much, I'm ok with cleanup and reorganization as you suggested.

Do you want me to fully respect the parent node? For example, don't attempt to indent to the "correct" position if the parent were correctly indented, indent to the current indent parent only. If yes, then I'll have to figure out a different solution of some sort over in #627.

I'd prefer having it always respect the parent node, since that's how Emacs works. But if you think it will drastically help to leave it as it is, I'm ok with that. I don't want to cause overcomplication for something that might not be an issue 99% of the time. I think most of the users either reindent the whole thing or nothing, not just a single line item.

Do you want me to remove the dedent to match the blocks if the block content is overindented? Again, I can't think of a reason why you wouldn't want to be aligned to the block, but I'll do it if you prefer it this way.

No need, leave it as it is now. It works great.

[PriceHiller]

I was thinking about 2 separate functions because it will be probably easier to maintain later. I'm expecting that we will get more issue reports once the virtual indents kick in, so it might be difficult to distinguish the code that needs fixing, and might be harder to debug.
If you think that'll be too much, I'm ok with cleanup and reorganization as you suggested.

I'll do some reorg then and ping you when it's ready for additional review. I have some things in mind now that you mentioned that. I think you'll find debugging what I have in mind to be relatively easy. I'm not 100% certain what I have in mind is viable, but I'm gonna give it a shot and find out.

[PriceHiller]

I'd prefer having it always respect the parent node, since that's how Emacs works. But if you think it will drastically help to leave it as it is, I'm ok with that. I don't want to cause overcomplication for something that might not be an issue 99% of the time. I think most of the users either reindent the whole thing or nothing, not just a single line item.

It helps a lot with #627. It was the easiest way I could think of at that moment that would make the virtual indents play nice without a mountain of additional code.

If you can think of something that could be done, I'm down to implement it. It's just the solution I thought of.

[PriceHiller]

I would like to die now. Of course! Of course it works on v0.9.0 and nightly, but not anything between.

[PriceHiller]

The tests pass around half the time on those non nightly versions... this is gonna be weird!.

[PriceHiller]

Alright, that's been resolved.

@kristijanhusak this should be ready for another review from you. I did a small smattering of reorganization to the indents. Should be a bit easier to parse through.

Oh, also:

I'd prefer having it always respect the parent node, since that's how Emacs works. But if you think it will drastically help to leave it as it is, I'm ok with that. I don't want to cause overcomplication for something that might not be an issue 99% of the time. I think most of the users either reindent the whole thing or nothing, not just a single line item.

This is back in for lists. The lists now adjust to the parent node again, not the "corrected" indentation for the parent.

[PriceHiller]

@kristijanhusak Not sure if you saw my ping. This is ready for you to take another look as a heads up.

If this is good I can incorporate these changes into the virtual indent PR.

[kristijanhusak]

Hey, sorry for the delayed review. I went through the code but didn't give it a proper test. Let's just address this one comment and I hope to get to properly reviewing it on Monday. Thanks!

[PriceHiller]

Hey, sorry for the delayed review. I went through the code but didn't give it a proper test. Let's just address this one comment and I hope to get to properly reviewing it on Monday. Thanks!

All good, I'll take a look at the comment and get it resolved. Thanks for taking a look 🙂

[kristijanhusak]

Everything looks great now, thanks!

[PriceHiller]

Awesome! I'll get #627 rebased on top of this when I get a moment. Most likely later in the week.