-
Couldn't load subscription status.
- Fork 237
Add preceding newlines before Doxygen commands #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1626,4 +1626,58 @@ class MarkupFormatterMixedContentTests: XCTestCase { | |
| ] | ||
| zip(expected, printed).forEach { XCTAssertEqual($0, $1) } | ||
| } | ||
|
|
||
| func testDoxygenCommandsPrecedingNewlinesWithSingleNewline() { | ||
| let expected = #""" | ||
| Does something. | ||
|
|
||
| \abstract abstract | ||
| \param x first param | ||
| \returns result | ||
| \note note | ||
| \discussion discussion | ||
| """# | ||
|
|
||
| let formattingOptions = MarkupFormatter.Options( | ||
| adjacentDoxygenCommandsSpacing: .singleNewline) | ||
| let printed = Document( | ||
| Paragraph(Text("Does something.")), | ||
| DoxygenAbstract(children: Paragraph(Text("abstract"))), | ||
| DoxygenParameter(name: "x", children: Paragraph(Text("first param"))), | ||
| DoxygenReturns(children: Paragraph(Text("result"))), | ||
| DoxygenNote(children: Paragraph(Text("note"))), | ||
| DoxygenDiscussion(children: Paragraph(Text("discussion"))) | ||
| ).format(options: formattingOptions) | ||
|
|
||
| XCTAssertEqual(expected, printed) | ||
| } | ||
|
|
||
| func testDoxygenCommandsPrecedingNewlinesAsSeparateParagraphs() { | ||
| let expected = #""" | ||
| Does something. | ||
|
|
||
| \abstract abstract | ||
|
|
||
| \param x first param | ||
|
|
||
| \returns result | ||
|
|
||
| \note note | ||
|
|
||
| \discussion discussion | ||
| """# | ||
|
|
||
| let formattingOptions = MarkupFormatter.Options( | ||
| adjacentDoxygenCommandsSpacing: .separateParagraphs) | ||
| let printed = Document( | ||
| Paragraph(Text("Does something.")), | ||
| DoxygenAbstract(children: Paragraph(Text("abstract"))), | ||
| DoxygenParameter(name: "x", children: Paragraph(Text("first param"))), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How would this behave with two different parameters? I wouldn't expect that to have blank lines in-between. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same reasoning as above. |
||
| DoxygenReturns(children: Paragraph(Text("result"))), | ||
| DoxygenNote(children: Paragraph(Text("note"))), | ||
| DoxygenDiscussion(children: Paragraph(Text("discussion"))) | ||
| ).format(options: formattingOptions) | ||
|
|
||
| XCTAssertEqual(expected, printed) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these need a blank line in between or would they parse the same if they were just on their own line (without blank lines in between)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They'd parse the same without a blank line.
A non-Doxygen-aware markdown renderer would show them on the same line without the blank line though (and most LSP clients like VS Code aren't Doxygen-aware).
If you don't think this is something swift-markdown needs to handle, would it be okay to add an option to always add a blank between Doxygen commands? If not, what do you recommend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what i've been able to see, Doxygen commands (and HeaderDoc tags, which are similar) are usually written without an interleaving newline between commands, especially commands of the same kind like
\param. Comparing this behavior "when parsed by a non-Doxygen-aware parser" doesn't seem that important to me, since Doxygen commands are non-standard Markdown to begin with.I would much rather prefer to see "preceding newlines" as a configurable option, with 1 newline (i.e. without an interleaving blank line) as the default. (Maybe as a toggle, something like "print Doxygen commands as separate paragraphs", where turning it on has the behavior you've written, and leaving it off only prints one newline.) You did find a valuable bug in that we don't print any newlines for Doxygen commands at all, so i would want to make sure that we at least print them correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@QuietMisdreavus @d-ronnqvist That makes sense. I updated the implementation to control this behavior with an option. Please recheck. 🙏🏼