Refactor indentexpr() to fix noindent indentation for lists.
#597
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
broken-pen
force-pushed
the
fix/indent-lists
branch
2 times, most recently
from
1d87dd5 to
b4fd73e
Compare
broken-pen
force-pushed
the
fix/indent-lists
branch
from
b4fd73e to
03272d1
Compare
broken-pen
force-pushed
the
fix/indent-lists
branch
3 times, most recently
from
0836945 to
77ea1d1
Compare
|
As far as I can tell, the latest test failure is a fluke. I can't seem to rerun the test though. Fail || Todo mappings should add last repeat property and state change to drawer (org_log_into_drawer)
.../orgmode/orgmode/tests/plenary/ui/mappings/todo_spec.lua:114: Expected objects to be the same.
- *[4] = ' :LAST_REPEAT: [2023-10-01 Sun 23:00]'
+ *[4] = ' :LAST_REPEAT: [2023-10-01 Sun 23:01]' |
kristijanhusak
approved these changes
Oct 7, 2023
There was a problem hiding this comment.
This looks good to me, thanks! @jgollenz can you take a look at this and confirm it fixes the issue you reported?
The tests replicate the current behavior, including all bugs. The goal is to update them as the bugs get fixed.
This simply turns the indentation line number and the current mode into optional arguments. If they're not passed, `vim.v.lnum` and `vim.fn.mode()` are used as usual. However, this allows calling a *hyptothetical* `indentexpr()` from the command line for debugging. Note that `indentexpr()` currently doesn't *use* `vim.fn.mode()`. It will in a future commit, though.
Closes nvim-orgmode#473. I tried doing small fixes to this code, but kept running into edge cases. Hence, this complete rewrite. :) The important points: - The queries in `indent.scm` no longer match on top-level (i.e un-nested) lists, but instead on list items of all levels. - List item indentation no longer relies on the previous non-empty line. Each list item stores whether it's in a top-level or a nested list and calculates its indent based on that. - The check whether we are in bulleted line or not no longer uses `str.match()`, since its pattern was buggy and forgot a few kinds of bullets. (namely, indented `*` bullets and `a.` ordered bullets) Instead, we compare the current line number to `match.line_nr`. We can do that because we query list items instead of lists now. There is an edge case when the user is appending to a list. We want that next line to be indented (see nvim-orgmode#472), but it's technically outside of the list. At the same time, if an unindented line follows a list, it should not become part of the list. The best solution I found for this was to make the behavior of `indentexpr()` depend on whether we are in insert mode. If yes, the line after a list is part of the list. If not, it isn't. The new code also correctly takes into account that two consecutive empty lines always end a preceding list.
broken-pen
force-pushed
the
fix/indent-lists
branch
from
77ea1d1 to
31fb1e0
Compare
|
Thanks for the PR @troiganto! @jgollenz if you find some issue regarding this PR please open up a separate issue, thanks! |
SlayerOfTheBad
pushed a commit
to SlayerOfTheBad/orgmode
that referenced
this pull request
Aug 16, 2024
…m-orgmode#597) Co-authored-by: troiganto <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #473.
I tried doing small fixes to this code, but kept running into edge cases. Hence, this complete rewrite. :) The important points:
indent.scmno longer match on top-level (i.e un-nested) lists, but instead on list items of all levels.str.match(), since its pattern was buggy and forgot a few kinds of bullets. (namely, indented*bullets anda.ordered bullets) Instead, we compare the current line number tomatch.line_nr. We can do that because we query list items instead of lists now.There is an edge case when the user is appending to a list. We want that next line to be indented (see #472), but it's technically outside of the list. At the same time, if an unindented line follows a list, it should not become part of the list.
The best solution I found for this was to make the behavior of
indentexpr()depend on whether we are in insert mode. If yes, the line after a list is part of the list. If not, it isn't.The new code also correctly takes into account that two consecutive empty lines always end a preceding list.
Let me know if there are any changes that should be made. It's kind of a big PR, so I assume it'll take a while to review. Thanks as always for maintaining this plugin! 👍