Add Virtual Indent #627

PriceHiller · 2023-11-09T19:47:39Z

This is largely a carry on from #561 with additional fixes and logic. I'm assuming that PR is likely dead in the water seeing as no further progress has been made since May.

Notes:

We must use nvim_buf_attach for the on_lines callback in virtual_indent.lua. Ephemeral marks with inline virtual text is not supported. It was accidentally supported by Neovim for a few dev versions previously. As such we have to use nvim_buf_attach so we can call nvim_buf_del_extmark. Using nvim_set_decoration_provider will error with nvim_buf_del_extmark.
I changed org_indent_mode to a pure boolean value like in Emacs Orgmode. I added a simple wrapper to handle folks who have noindent and indent in their configs set to that var.
I implemented support for realigning items in promotions and deletions.
You'll notice that I have a hack as part of the on_lines callback in virtual_indent.lua. This doesn't cause any weird behavior, but it sure does look messy. I had to do that to avoid some issues with the on_lines callback being triggered early (as was the case in feat(ui): virtual indent #561).
This is the one where I'm stumped. Promotions and demotions on already indented lists (the list root isn't aligned at column 0) does not fully reindent that list. It only reindents the root list node and subsequent demotions & promotions reindent down the subnodes until it's all flush. Not sure how to fix this. Would like some advice here for sure. It's not a big deal imo, it's simple enough to format the lines with visual mode, but I'm hoping to avoid that as it causes the cursor to move during a promotion/demotion.

Problem exposed in point 5 has been resolved by a rebase on top of #629

@kristijanhusak Let me know what you think, and I'd really like some feedback on the fifth note there. Once I can resolve the fifth note and you like the core of this, then I'll promote this to a full PR. 🙂

PriceHiller · 2023-11-09T19:50:40Z

Cannot replicate the test failures above... works as expected locally. Seems the todo_spec is exploding because the timestamp is ever so slightly different on the properties.

PriceHiller · 2023-11-10T23:12:13Z

Resolved test failures, fixed some logic errors. Mostly to do with respecting "legacy" behavior in promotions & demotions as well as correctly checking vim.version() to enable VirtualIndent.

PriceHiller · 2023-11-15T05:51:13Z

Rebased on top of #629

PriceHiller · 2023-11-15T06:03:01Z

~~TODO: Write tests to validate virt cols and then this should be ready.~~

Done.

PriceHiller · 2023-11-15T08:05:01Z

@kristijanhusak

I'll mark this ready for review if #629 gets merged. If it doesn't, then I'll need to undo my rebase here and that will reintroduce the issues in the fifth point. As is it stands though, if #629 gets merged this should be good on point 5.

PriceHiller · 2023-12-26T22:02:32Z

I realized I totally forgot about this! I had a reminder pop up on my phone today.

I'm going to pick up where I left off some point this week and update/fix what needs fixing with any changes on master so this can get integrated. My fault, totally forgot. Dementia sets in early apparently.

PriceHiller · 2023-12-27T18:57:43Z

I am unable to replicate the test failures occurring here in the CI. When using Neovim version 0.9.0 & 0.9.1 the tests pass as expected, even on multiple runs.

PriceHiller · 2023-12-27T19:18:49Z

@kristijanhusak Do you have any ideas as to why these tests are inconsistent? I'm kinda grasping at straws here -- even recompiling Neovim now for the failed version(s) makes those tests work as expected locally.

The moment I can get these tests playing nice I can probably promote this from draft status.

kristijanhusak · 2023-12-27T20:07:59Z

@treatybreaker I re-ran them now manually and it passed. I'm not sure why it's not consistent. We have some random failures in case tests overlap between 2 minutes and assertion fails, but these failed because some indentation was not properly done in the assertion. See if you can reproduce it locally by running tests multiple times in a row.

PriceHiller · 2023-12-27T20:20:24Z

See if you can reproduce it locally by running tests multiple times in a row.

I can with the following script:

TEST_RUN=1
while :; do
  printf "==== Current Test Run: %s ====\n" "${TEST_RUN}"
  if make test; then
    printf "\n\n++++ Finished Test Run: %s ++++\n\n" "${TEST_RUN}"
  else
    printf "\n\n==== Tests Failed on Run %s ====\n\n" "${TEST_RUN}"
    break
  fi
  ((TEST_RUN++))
done

Might need to run for a while. It's always the same test that fails: with "indent", "0gg=G" reindents the whole file. I wonder if the test file is loaded fully or if Orgmode is 100% ready when the full reindent is called. Seems to be inconsistent if it fails, but I'm not 100% certain as to why.

Definitely an issue with this PR, I've let that script run for a while on the master branch -- no issues. Gonna dig in and find out what's going on.

PriceHiller · 2023-12-27T21:14:05Z

Correction, I can cause this failure consistently on master.

kristijanhusak · 2023-12-27T21:15:36Z

@treatybreaker I'm assuming your previous PR causes the issue #629 . Can you check there what's happening?

PriceHiller · 2023-12-28T05:06:02Z

With the seeming resolution of the bug above in #640, I'm going to promote this from a draft.

I'm ready for some nitpicks.

kristijanhusak

Thanks for the PR. I gave this a quick test, it looks ok, but I will test it thoroughly once we align on the comment below.
org_indent_mode in here is a bit confusing at this moment. This is how I initially thought it should work:

indent means that it should apply real indentation to everything with spaces, as it is doing now by default
noindent - Do not apply any indentations with spaces, and let the virtual indentation do the indentation.

I'm aware this is not correct so we should follow what Emacs does. This is how it is actually enabled from the configuration in Emacs: https://stackoverflow.com/a/66793922

DOCS.md

PriceHiller · 2024-01-17T17:58:54Z

Hey, I'll do my best to respond and resolve some issues in this PR on Friday. Been quite busy recently. Just a heads up 🙂.

kristijanhusak · 2024-01-17T18:10:03Z

No worries. There's no rush on this, it's only on nightly anyway 😄

PriceHiller · 2024-01-24T23:27:42Z

The latest commit did a refactor to make org_adapt_indentation responsible for hard indentation similar to Emacs.

Not sure if I need to add something that disables it when org_startup_indent is enabled.

Copying from a previous reply of mine as to how org_adapt_indentation now works:

When org_startup_indented is disabled and org_adapt_indentation is enabled you only get hard indentation, just like on master currently.

When org_startup_indented is enabled and org_adapt_indentation is disabled you get only Virtual Indentation.

When org_startup_indented is true and org_adapt_indentation is enabled you get both Virtual indentation and hard indents like Emacs.

kristijanhusak

I gave it a test, it works great, and code looks good overall.
I only noticed one inconsistency.

If org_startup_indented = true and org_adapt_indentation = true, when I open up the file and reindent the file, it doesn't change any indentation.

For example, I have this content that is hard indented:

* TODO Test
  Test
  List
  - First
  - second
    - level2
      - level3
    - level2 item

Now when I apply above configuration (setting both to true) and open it, I get this:

virtual indent + hard indent:

* TODO Test
    Test
    List
    - First
    - second
      - level2
        - level3
      - level2 item

This is working as expected. But now, if I do gg=G, no indentation is actually changed.
I would expect to realign everything so it adapts to the virtual indentation.
It does that when org_adapt_indentation = false, so it generally works. We just need to tweak the checks.

AFAIK Emacs doesn't have this idea of (hard) reindentation, so this is something we can figure out ourselves.

If we follow the default Emacs settings for org indent mode I mentioned previously link, which states that when org-indent-mode is enabled on buffer, it disables the org-adapt-indentation, we can expect to apply the reindentation logic I mentioned above.

So org_startup_indented always has higher priority over org_adapt_indentation, until we maybe introduce org_indent_mode_turns_off_org_adapt_indentation mentioned here, but we'll leave that as an improvement for later.

DOCS.md

PriceHiller · 2024-01-25T13:28:03Z

@kristijanhusak

I'm on mobile currently so I can't make any changes; however, if you want to wait until 2 - 3pm CST I can add in disabling adapt indentation when indent mode is enabled by default along with the new flag you mention.

If not, I'm more than happy to just create a follow up PR with the functionality implemented then. LMK

kristijanhusak · 2024-01-25T13:35:15Z

I'm on mobile currently so I can't make any changes; however, if you want to wait until 2 - 3pm CST I can add in disabling adapt indentation when indent mode is enabled by default along with the new flag you mention.

Don't worry about this, work on it when you get a chance. Once we merge it in it will be considered experimental for some time until we flush out any potential issues. Ideally, I would put this out only when Neovim 0.10 comes out, but I don't think we can rely on that.

See nvim-orgmode#627 (review) > I gave it a test, it works great, and code looks good overall. I only noticed one inconsistency. > > If `org_startup_indented = true` and `org_adapt_indentation = true`, when I open up the file and reindent the file, it doesn't change any indentation. > > For example, I have this content that is hard indented: > > ``` > * TODO Test > Test > List > - First > - second > - level2 > - level3 > - level2 item > ``` > > Now when I apply above configuration (setting both to `true`) and open it, I get this: > > virtual indent + hard indent: > > ``` > * TODO Test > Test > List > - First > - second > - level2 > - level3 > - level2 item > ``` > > This is working as expected. But now, if I do gg=G, no indentation is actually changed. I would expect to realign everything so it adapts to the virtual indentation. It does that when `org_adapt_indentation = false`, so it generally works. We just need to tweak the checks. > > AFAIK Emacs doesn't have this idea of (hard) reindentation, so this is something we can figure out ourselves. > > If we follow the default Emacs settings for org indent mode I mentioned previously [link](https://orgmode.org/manual/Org-Indent-Mode.html#:~:text=By%20default%2C%20Org%20Indent%20mode%20turns%20off%20org%2Dadapt%2Dindentation), which states that when org-indent-mode is enabled on buffer, it disables the `org-adapt-indentation`, we can expect to apply the reindentation logic I mentioned above. > > So `org_startup_indented` always has higher priority over `org_adapt_indentation`, until we maybe introduce `org_indent_mode_turns_off_org_adapt_indentation` mentioned [here](https://orgmode.org/manual/Org-Indent-Mode.html#:~:text=If%20you%20want%20to%20customize%20this%20default%20behavior%2C%20see%20org%2Dindent%2Dmode%2Dturns%2Don%2Dhiding%2Dstars%20and%20org%2Dindent%2Dmode%2Dturns%2Doff%2Dorg%2Dadapt%2Dindentation.), but we'll leave that as an improvement for later.

PriceHiller · 2024-01-25T19:28:23Z

@kristijanhusak

org_startup_indented now disables org_adapt_indentation by default, with a configuration option org_indent_mode_turns_off_org_adapt_indentation that is enabled by default. If that is set to false, then org_adapt_indentation won't be disabled when org_startup_indented is used.

AFAIK Emacs doesn't have this idea of (hard) reindentation, so this is something we can figure out ourselves.

In Emacs, if I set org-adapt-indentation to headline-data then the behavior you're seeing with the hard indents and virtual indents is identical to how it's implemented in this PR. Same thing happens if I set org-adapt-indentation to t in Emacs as well.

Not sure if I need to do anything else around indent adaption seeing as it follows Emacs now from what I can tell.

PriceHiller · 2024-01-25T19:32:41Z

What else needs to be done for the Virtual Indents?

Off the top of my head:

Toggling indent mode on or off in a buffer
~~Toggling org_adapt_indentation on or off in a buffer~~ Not supported, Emacs only has this by changing a config inline in the first place.
Support #+STARTUP: indent & #+STARTUP: noindent

kristijanhusak

Everything works great now! Just some minor changes.

Regarding the changes, I'm not sure about this:

Toggling org_adapt_indentation on or off in a buffer

Does Emacs support this? I know it can toggle org-indent-mode, but I don't think it can change this setting in the same way.

t.org

kristijanhusak · 2024-01-25T19:48:27Z

ftplugin/org.lua

@@ -10,6 +10,12 @@ config:setup_mappings('org')
 config:setup_mappings('text_objects')
 config:setup_foldlevel()

+if config.org_startup_indented then
+  config.org_adapt_indentation = not config.org_indent_mode_turns_off_org_adapt_indentation


Let's move this line to Config:extend where we also do some validation and changes to the config.

Will do. I do have to keep the conditional there though, as we use the ftplugin to set the buffer org_indent_mode variable as a heads up.

Accidentally included in a `git add` operation. See nvim-orgmode#627 (comment) 😅

See nvim-orgmode#627 (comment)

PriceHiller · 2024-01-25T20:05:41Z

Does Emacs support this? I know it can toggle org-indent-mode, but I don't think it can change this setting in the same way.

Open any org file in emacs and enable org-indent-mode. Then you can use <M-x>eval-expression to evalute (setq org-adapt-indentation t).

That's the "toggle" I'm talking about.

kristijanhusak · 2024-01-25T20:08:39Z

I don't think we need to worry about this. That's changing a config inline.

Just one more thing please. Let's squash the commits into 1 or 2. There are > 30 commits , so it will not look helpful once someone looks at it in the changes.

Allows Virtual indentation to be used simmilar to emacs `org-indent-mode` by adding virtual spaces before text under headlines to align them with a given headline. This adds the following new options mirroring Emacs: - `org_startup_indented` - `org_adapt_indentation` - `org_indent_mode_turns_off_org_adapt_indentation` And deprecates the config option `org_indent_mode`. A shim was added for backwards compatibility so this is not a breaking change. See `DOCS.md` for additional details.

PriceHiller · 2024-01-25T20:16:26Z

@kristijanhusak

Squashed all down into one commit. Sorry about how many there were 😅. Lots of tiny things got put together and I had to rebase/merge things done on master a few times.

kristijanhusak

Ok, I think this is good to go.
Thank you very much! You did some huge work here.

PriceHiller · 2024-01-25T20:54:52Z

@kristijanhusak

As a heads up, I'm sure folks are going to find issues that I haven't been able to. I've been daily driving off this branch since I opened this PR and it's worked relatively well for me, but I'm only one person with a single config.

If an issue comes in for the virtual indents, I'm not gonna leave you hanging -- ping me (@treatybreaker) on those issues I'll make them my problem.

This commit makes `headlines.nvim` compatible with Nvim Orgmode's new Virtual Indent option added by nvim-orgmode/orgmode#627. This also will ensure any features like it in the future from other plugins do not interfere with the headlines later on. In theory, extmark priorities would work here, but in pratice they do not. If this extmark is set *last* by a higher priority using only the virtual text, it can be arbitrarily shifted by other extmark content and the same occurs when the extmark set is of a lower priority. By specifying the window column to apply to, we bypass all the priorities shenanigans and directly set the correct position of the headline.

Allows Virtual indentation to be used simmilar to emacs `org-indent-mode` by adding virtual spaces before text under headlines to align them with a given headline. This adds the following new options mirroring Emacs: - `org_startup_indented` - `org_adapt_indentation` - `org_indent_mode_turns_off_org_adapt_indentation` And deprecates the config option `org_indent_mode`. A shim was added for backwards compatibility so this is not a breaking change. See `DOCS.md` for additional details.

PriceHiller force-pushed the feat/virtual-indent branch 2 times, most recently from d8652f6 to 995a52e Compare November 10, 2023 23:10

PriceHiller mentioned this pull request Nov 15, 2023

fix: improve block and list indentations #629

Merged

PriceHiller force-pushed the feat/virtual-indent branch from 862875e to 55b750e Compare November 15, 2023 05:50

PriceHiller force-pushed the feat/virtual-indent branch 3 times, most recently from e4dcf96 to 4992415 Compare November 15, 2023 08:02

PriceHiller mentioned this pull request Nov 22, 2023

Allow listitem folding #630

Open

PriceHiller force-pushed the feat/virtual-indent branch 3 times, most recently from 8a543db to a3b4a6c Compare December 27, 2023 18:52

PriceHiller force-pushed the feat/virtual-indent branch from a3b4a6c to 44bd3ba Compare December 27, 2023 18:59

PriceHiller mentioned this pull request Dec 27, 2023

Tests fail for full reindents with indent mode on non-nightly (0.10.0) Neovim #639

Closed

PriceHiller marked this pull request as ready for review December 28, 2023 05:06

kristijanhusak requested changes Jan 15, 2024

View reviewed changes

DOCS.md Show resolved Hide resolved

PriceHiller force-pushed the feat/virtual-indent branch 2 times, most recently from 47b5dcd to 6686aea Compare January 24, 2024 23:21

PriceHiller requested a review from kristijanhusak January 24, 2024 23:25

kristijanhusak requested changes Jan 25, 2024

View reviewed changes

DOCS.md Outdated Show resolved Hide resolved

kristijanhusak requested changes Jan 25, 2024

View reviewed changes

PriceHiller added a commit to PriceHiller/orgmode that referenced this pull request Jan 25, 2024

chore: remove loose file

cac865a

Accidentally included in a `git add` operation. See nvim-orgmode#627 (comment) 😅

PriceHiller added a commit to PriceHiller/orgmode that referenced this pull request Jan 25, 2024

refactor: move auto setting org_adapt_indentation to Config:extend

2e8410c

See nvim-orgmode#627 (comment)

PriceHiller force-pushed the feat/virtual-indent branch from 2e8410c to d7d9ec7 Compare January 25, 2024 20:15

kristijanhusak approved these changes Jan 25, 2024

View reviewed changes

kristijanhusak merged commit 4f30b8f into nvim-orgmode:master Jan 25, 2024
5 checks passed

kristijanhusak mentioned this pull request Jan 25, 2024

feat(ui): virtual indent #561

Closed

2 tasks

PriceHiller deleted the feat/virtual-indent branch January 25, 2024 20:55

kristijanhusak mentioned this pull request Jan 25, 2024

Status of org-indent-mode functionality? #422

Open

This was referenced Jan 28, 2024

feat: make VirtualIndent dynamically attach/detach based on vim.b.org_indent_mode #658

Merged

feat: add org-indent-mode-turns-on-hiding-stars equivalent #659

Merged

PriceHiller mentioned this pull request Feb 24, 2024

fix: ensure fat headlines are not shifted by conflicting extmarks lukas-reineke/headlines.nvim#70

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Virtual Indent #627

Add Virtual Indent #627

PriceHiller commented Nov 9, 2023 •

edited

Loading

PriceHiller commented Nov 9, 2023

PriceHiller commented Nov 10, 2023

PriceHiller commented Nov 15, 2023

PriceHiller commented Nov 15, 2023 •

edited

Loading

PriceHiller commented Nov 15, 2023

PriceHiller commented Dec 26, 2023

PriceHiller commented Dec 27, 2023

PriceHiller commented Dec 27, 2023

kristijanhusak commented Dec 27, 2023

PriceHiller commented Dec 27, 2023 •

edited

Loading

PriceHiller commented Dec 27, 2023

kristijanhusak commented Dec 27, 2023

PriceHiller commented Dec 28, 2023

kristijanhusak left a comment

PriceHiller commented Jan 17, 2024

kristijanhusak commented Jan 17, 2024

PriceHiller commented Jan 24, 2024 •

edited

Loading

kristijanhusak left a comment

PriceHiller commented Jan 25, 2024

kristijanhusak commented Jan 25, 2024

PriceHiller commented Jan 25, 2024

PriceHiller commented Jan 25, 2024 •

edited

Loading

kristijanhusak left a comment

kristijanhusak Jan 25, 2024

PriceHiller Jan 25, 2024

PriceHiller commented Jan 25, 2024

kristijanhusak commented Jan 25, 2024

PriceHiller commented Jan 25, 2024

kristijanhusak left a comment

PriceHiller commented Jan 25, 2024

Add Virtual Indent #627

Add Virtual Indent #627

Conversation

PriceHiller commented Nov 9, 2023 • edited Loading

PriceHiller commented Nov 9, 2023

PriceHiller commented Nov 10, 2023

PriceHiller commented Nov 15, 2023

PriceHiller commented Nov 15, 2023 • edited Loading

PriceHiller commented Nov 15, 2023

PriceHiller commented Dec 26, 2023

PriceHiller commented Dec 27, 2023

PriceHiller commented Dec 27, 2023

kristijanhusak commented Dec 27, 2023

PriceHiller commented Dec 27, 2023 • edited Loading

PriceHiller commented Dec 27, 2023

kristijanhusak commented Dec 27, 2023

PriceHiller commented Dec 28, 2023

kristijanhusak left a comment

Choose a reason for hiding this comment

PriceHiller commented Jan 17, 2024

kristijanhusak commented Jan 17, 2024

PriceHiller commented Jan 24, 2024 • edited Loading

kristijanhusak left a comment

Choose a reason for hiding this comment

PriceHiller commented Jan 25, 2024

kristijanhusak commented Jan 25, 2024

PriceHiller commented Jan 25, 2024

PriceHiller commented Jan 25, 2024 • edited Loading

kristijanhusak left a comment

Choose a reason for hiding this comment

kristijanhusak Jan 25, 2024

Choose a reason for hiding this comment

PriceHiller Jan 25, 2024

Choose a reason for hiding this comment

PriceHiller commented Jan 25, 2024

kristijanhusak commented Jan 25, 2024

PriceHiller commented Jan 25, 2024

kristijanhusak left a comment

Choose a reason for hiding this comment

PriceHiller commented Jan 25, 2024

PriceHiller commented Nov 9, 2023 •

edited

Loading

PriceHiller commented Nov 15, 2023 •

edited

Loading

PriceHiller commented Dec 27, 2023 •

edited

Loading

PriceHiller commented Jan 24, 2024 •

edited

Loading

PriceHiller commented Jan 25, 2024 •

edited

Loading