code-action: TOC improvements #22
Conversation
Otherwise they're not interpreted as nested lists and not rendered correctly in, say, HTML
For example, after YAML frontmatter, after first title Additionally, correctly render TOCs that start from a heading of size different to 1
|
@keynmol I checked out locally to try things out. I think something got lost during rebase, as all this insertion logic from CodeActions.fs is not wired into Server.fs and is used only inside tests. |
|
@artempyanykh This should be ready for review/another manual test |
|
@keynmol could you rebase on the latest main please? |
|
Done |
|
|
| >> Log.addContext "text" rendered | ||
| ) | ||
|
|
||
| let isEmpty lineNumber = document.text.LineContent(lineNumber).Trim().Length.Equals(0) |
There was a problem hiding this comment.
nit: Lenth.Equals(0) -> IsEmpty?
There was a problem hiding this comment.
The type 'String' does not define the field, constructor or member 'IsEmpty'
Do I need to open anything or bribe someone to get it?
There was a problem hiding this comment.
System.String.IsNullOrEmpty. This is a bit ridiculous though. We should add an extension method to String at some point.
|
|
||
| let modifiedText = applyDocumentAction doc action | ||
|
|
||
| let expected = |
There was a problem hiding this comment.
nit: I'm not sure whether it'll be better in this case, but you could try snapshot tests here to avoid manually typing all these expected doc contents.
|
|
|
Awesome! I did a bit of testing and it worked great. There was just one case where TOC ate a new line LMK if you'd like to merge as is or fix the issue. |
|
I'd rather merge as is (as long as it doesn't lines repeatedly), and then add this case to the tests with some other testing infra improvements |
|
|
Closes #19
TODO