Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion Sources/Markdown/Walker/Walkers/MarkupFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ fileprivate extension Markup {
}
return nil
}

/// Previous sibling of this element in its parent, or `nil` if it's the first child.
var previousSibling: Markup? {
guard let parent, indexInParent > 0 else {
return nil
}

return parent.child(at: indexInParent - 1)
}

/// Whether this element is a Doxygen command.
var isDoxygenCommand: Bool {
return self is DoxygenDiscussion || self is DoxygenNote || self is DoxygenAbstract
|| self is DoxygenParameter || self is DoxygenReturns
}
}

fileprivate extension String {
Expand Down Expand Up @@ -239,6 +254,15 @@ public struct MarkupFormatter: MarkupWalker {
case at = "@"
}

/// The spacing to use when formatting adjacent Doxygen commands.
public enum AdjacentDoxygenCommandsSpacing: String, CaseIterable {
/// Separate adjacent Doxygen commands with a single newline.
case singleNewline = "single-newline"

/// Keep a blank line between adjacent Doxygen commands creating separate paragraphs.
case separateParagraphs = "separate-paragraphs"
}

// MARK: Option Properties

var orderedListNumerals: OrderedListNumerals
Expand All @@ -253,6 +277,7 @@ public struct MarkupFormatter: MarkupWalker {
var preferredLineLimit: PreferredLineLimit?
var customLinePrefix: String
var doxygenCommandPrefix: DoxygenCommandPrefix
var adjacentDoxygenCommandsSpacing: AdjacentDoxygenCommandsSpacing

/**
Create a set of formatting options to use when printing an element.
Expand All @@ -270,6 +295,7 @@ public struct MarkupFormatter: MarkupWalker {
- preferredLineLimit: The preferred maximum line length and method for splitting ``Text`` elements in an attempt to maintain that line length.
- customLinePrefix: An addition prefix to print at the start of each line, useful for adding documentation comment markers.
- doxygenCommandPrefix: The command command prefix, which defaults to ``DoxygenCommandPrefix/backslash``.
- adjacentDoxygenCommandsSpacing: The spacing to use when formatting adjacent Doxygen commands.
*/
public init(unorderedListMarker: UnorderedListMarker = .dash,
orderedListNumerals: OrderedListNumerals = .allSame(1),
Expand All @@ -282,7 +308,8 @@ public struct MarkupFormatter: MarkupWalker {
preferredHeadingStyle: PreferredHeadingStyle = .atx,
preferredLineLimit: PreferredLineLimit? = nil,
customLinePrefix: String = "",
doxygenCommandPrefix: DoxygenCommandPrefix = .backslash) {
doxygenCommandPrefix: DoxygenCommandPrefix = .backslash,
adjacentDoxygenCommandsSpacing: AdjacentDoxygenCommandsSpacing = .singleNewline) {
self.unorderedListMarker = unorderedListMarker
self.orderedListNumerals = orderedListNumerals
self.useCodeFence = useCodeFence
Expand All @@ -297,6 +324,7 @@ public struct MarkupFormatter: MarkupWalker {
self.thematicBreakLength = max(3, thematicBreakLength)
self.customLinePrefix = customLinePrefix
self.doxygenCommandPrefix = doxygenCommandPrefix
self.adjacentDoxygenCommandsSpacing = adjacentDoxygenCommandsSpacing
}

/// The default set of formatting options.
Expand Down Expand Up @@ -1173,28 +1201,47 @@ public struct MarkupFormatter: MarkupWalker {
print(formattingOptions.doxygenCommandPrefix.rawValue + name + " ", for: element)
}

private mutating func ensureDoxygenCommandPrecedingNewline(for element: Markup) {
guard let previousSibling = element.previousSibling else {
return
}

guard formattingOptions.adjacentDoxygenCommandsSpacing == .singleNewline else {
ensurePrecedingNewlineCount(atLeast: 2)
return
}

let newlineCount = previousSibling.isDoxygenCommand ? 1 : 2
ensurePrecedingNewlineCount(atLeast: newlineCount)
}

public mutating func visitDoxygenDiscussion(_ doxygenDiscussion: DoxygenDiscussion) {
ensureDoxygenCommandPrecedingNewline(for: doxygenDiscussion)
printDoxygenStart("discussion", for: doxygenDiscussion)
descendInto(doxygenDiscussion)
}

public mutating func visitDoxygenAbstract(_ doxygenAbstract: DoxygenAbstract) {
ensureDoxygenCommandPrecedingNewline(for: doxygenAbstract)
printDoxygenStart("abstract", for: doxygenAbstract)
descendInto(doxygenAbstract)
}

public mutating func visitDoxygenNote(_ doxygenNote: DoxygenNote) {
ensureDoxygenCommandPrecedingNewline(for: doxygenNote)
printDoxygenStart("note", for: doxygenNote)
descendInto(doxygenNote)
}

public mutating func visitDoxygenParameter(_ doxygenParam: DoxygenParameter) {
ensureDoxygenCommandPrecedingNewline(for: doxygenParam)
printDoxygenStart("param", for: doxygenParam)
print("\(doxygenParam.name) ", for: doxygenParam)
descendInto(doxygenParam)
}

public mutating func visitDoxygenReturns(_ doxygenReturns: DoxygenReturns) {
ensureDoxygenCommandPrecedingNewline(for: doxygenReturns)
// FIXME: store the actual command name used in the original markup
printDoxygenStart("returns", for: doxygenReturns)
descendInto(doxygenReturns)
Expand Down
54 changes: 54 additions & 0 deletions Tests/MarkdownTests/Visitors/MarkupFormatterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +1659 to +1667
Copy link
Contributor

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)?

\abstract ...
\param x ...
\returns ...
...

Copy link
Member Author

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?

Copy link
Contributor

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.

Copy link
Member Author

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. 🙏🏼

"""#

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"))),
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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)
}
}