Rewrite indentation code #31
Conversation
The objectives are:
1. Simplify the indentation code; previous implementation has become
so complex it is impossible to maintain,
2. Significantly improve performance; previous indentation code was
painfully slow, (see issue #6)
3. Maximum configurability; should be configured similarly to cljfmt
and make previously impossible things possible (e.g. issue #21).
As of this commit, objectives 1 and 2 have been met, but work on
objective 3 has not yet begun. There will continue to be further
improvements, particularly around performance and the "what if syntax
highlighting is disabled?" scenario.
These changes will unfortunately be backwards incompatible, but
hopefully the improved performance and API will make up for it.
By utilising some ugly mutable state, this change more than doubles the performance of the indentation code. You can expect even further benefits in larger code blocks. This should completely eliminate any need for the `clojure_maxlines` configuration option.
Since maps are far more likely to across multiple lines than vectors (and when they are they tend to be much larger, e.g. EDN files!), we can further optimise indentation by checking if the line is inside a map *before* checking it is inside a vector. (This happens because of the performance improvement made with the addition of the optimised `CheckPair` function.)
The `=` operator will no longer alter the indent of lines within a Clojure multi-line string or regular expression. The previous behaviour was annoying for writing detailed doc-strings, as it made reformatting the file with `gg=G` not possible as it would screw up the indentation within the doc-strings. Now the behaviour matches that of VS Code and Emacs.
By setting the `clojure_align_multiline_strings` option to `-1` it will switch to traditional Lisp multi-line string indentation, of no indent.
Sometimes it seems that when `=`ing over a block, Vim will sometimes not re-highlight strings correctly until we have already ran `searchpairpos`. In this case, we implement a hacky workaround which detects the false-positive and recovers from it.
Still lots to do here, this is just the initial work.
|
I've been experimenting with a custom algorithm as per the check list item above. This version eliminates all syntax highlight checks (and therefore no longer requires the various hacks to make those checks work) plus it is approx the same speed as my first rewrite attempt and uses about the same amount of code. However the core benefits of this version is that it works when syntax highlighting is switched off, and eliminates the performance bottle neck of syntax highlight checks. This means that it will be possible to write a version of the algorithm in Vim9 script, to get another significant performance improvement! (Maintaining both a legacy Vim script and Vim9 script versions should be feasible due to how much simpler this is than the original implementation.) TL;DR: I'll push a new version to this branch soon which will be:
Once this branch is feature complete, I'll share benchmarks comparing it to the original as is on |
Not complete yet, but getting there.
Some refactoring should be possible here and further optimisations. Once all optimisations I can think of have been implemented, I'll try writing an alternate Vim9 script version. (The syntax highlight group checks used in previous implementations of the indentation code was the core bottleneck, so a Vim9 script version would not have been much faster.)
The performance of the new reader-like indentation algorithm has now surpassed the performance of my previous syntax highlighting approach with better accuracy and no hacky code required.
|
Performance benchmarks as of current change (+ unpushed Vim9 version). This is measuring the time to reindent a whole file on a Macbook Pro 2021, so expect much more noticable performance gains on slower hardware. 🚀
(I'll share new benchmarks (and the full reports generated by Vim) once this branch is feature complete, although I don't expect it to change much.) Even with the Vim9 implementation in the code too (not pushed yet, still figuring out how best to integrate it in a maintainable way), we can expect the file size of Regarding Neovim, until Vim9 script support is added, the "New (legacy)" code will be used. If anyone would like to volunteer to write a Lua implementation of this code for Neovim please open an issue and we can discuss how to integrate it. |
Previously backslashes were accidentally detected as tokens by the indentation tokeniser. This meant that character literals, would break indentation of everything after them.
|
Thanks for working on this, I've been trying clojure dev in neovim lately and was getting really frustrated by the fact that it would indent based on brackets in string literals. I just tried this branch and that problem is fixed. |
Fixes indentation of reader conditional macros. May choose to remove this if function argument indentation is generally preferred by programmers when a keyword is in function position.
@SavageMessiah great to hear that! If you encounter any issues on this branch please let me know. |
|
Feedback: (filter-test
a b c)
(filter-suite
a b c)
(letfn [(filter-test
[test-case]
...)
(filter-suite
[suite]
...)]
...)It seems the matching algorithm for function indentation is based on any portion of the function's name, not the whole thing, so Additionally, (letfn [(func [test-case]
...)]
...)Thanks so much! |
|
Hi @NoahTheDuke, thanks for raising and testing!
Unfortunately I'm not seeing the same behaviour you are with
I am aware of this current deficiency in the indentation algorithm. It is possible to solve, but it will be tricky as I will fix it, but it will likely be after this PR is merged as their use isn't particularly common and hopefully the other improvements I've made will make up for it being a temporary issue. As a partial workaround, I'd recommend using Asides
Why is indenting
|
Vim script is infuriating sometimes! Turns out that the indentation tests locally passed with the previous code, but failed in CI and real life.
When the "uniform" indent style is selected, we no longer apply other
list indentation rules/analysis.
Unfortunately the reader conditional changes broke indentation for the
following:
(ns my-ns
(:require [clojure.string :as str]
[clojure.spec.alpha :as s]))
Indenting it as this:
(ns my-ns
(:require [clojure.string :as str]
[clojure.spec.alpha :as s]))
Which is incorrect.
Makes indentation more robust against users toggling the `'magic'` option.
This PR contains a rewrite of the whole Clojure indentation code with several aims:
Along with these aims, this PR makes several further quality-of-life improvements to the indentation system, such as making it such that
=does not alter the indentation of multi-line strings.As of the latest change, the indentation is over twice as fast as before and implemented in about half the code.
To do
Add the nosearchpairposfallback.=operator is used.Fix incorrect indentation caused by syntax glitches.~,~@,',@,#',#_, etc.).clojure_indent_rulesoption and implement it.#?@(...),#?(...)).letfn,extend-protocol, etc.).