Rework indentation algo and add tests - #50
Conversation
|
@abrochard the tests are failing because of the pre-existing change 😊 The "package-requires" tag is set to 25.3, however compilation on 25.3 doesn't work because the code uses |
|
The |
|
upd: new commit — added basic support for Btw, with this PR indentation support for |
929fdf6 to
75c9a1c
Compare
|
Hi @Hi-Angel , sorry for the delay and thank you for submitting this! |
75c9a1c to
97e2019
Compare
|
Thanks! upd: while reviewing my code I noticed I assumed we use Fun fact btw, this test fails for the old (i.e. current master) behavior for exactly same reason — it lacked a check for negative numbers somewhere. |
b8d1a86 to
f8c931f
Compare
Parsing whole buffer to calculate the current indentation is wasteful and error-prone. Mermaid evolves and adds new modes support for which may be absent from the mode yet, so the user may want to apply custom indentation and the plugin should account for it. It is very typical in indentation engines for various programming languages to account for previous indentation, because a user frequently may have a reason to align previous indentation to something only they aware of. The rule of thumb: if a user wants to reindent code prior to the current line, they can do that explicitly. So calculate current indentation based on assumption that previous indentation is already correct. This also fixes a bug (see tests) where nested subgraphs were indented linearly instead of increasing indentation, and another one with negative offsets. Fixes: abrochard#45
f8c931f to
cffff69
Compare
|
I tested locally and seems fine for the diagram types we support (ie. the simple ones). I haven't kept up and was surprised by all the kind of diagrams that mermaid now supports! |
|
@abrochard why did you squash the commits though? 🤔 |
Oh just force of habit |
|
Do you often squash commits…? |
Yeah all the time. I have a specific workflow at my job that requires it |
Rework indentation algorithm to prefer previous indentation
Parsing whole buffer to calculate the current indentation is wasteful
and error-prone. Mermaid evolves and adds new modes support for which
may be absent from the mode yet, so the user may want to apply custom
indentation and the plugin should account for it.
It is very typical in indentation engines for various programming
languages to account for previous indentation, because a user
frequently may have a reason to align previous indentation to
something only they aware of.
The rule of thumb: if a user wants to reindent code prior to the
current line, they can do that explicitly. So calculate current
indentation based on assumption that previous indentation is already
correct.
This also fixes a bug (see tests) where nested subgraphs were indented
linearly instead of increasing indentation; and another one with
negative numbers.
Fixes: #45
First commit adds indentation tests to make sure further indentation changes wouldn't break anything. At that point 3 tests are broken and are marked as such.
2nd commit reworks indentation algo per discussion in the issue, and that fixes the 3 tests. I also added more diagram-starter keywords that were missed in the older impl. such as
flowchart. Besides being useful on its own this will also simplify 3rd commit.3rd is a keywords deduplication.
Besides regression tests being added in the PR I also tested this algo on a large
graph TDI have locally. Before this commit attempting to re-indent it results in broken indentation (due to nested subgraphs bug, see tests 2nd commit), after this commit the buffer remains unchanged as expected.