Skip to content

add indentationStyle to MarkupFormatter.Options#204

Open
dpowers wants to merge 3 commits into
swiftlang:mainfrom
dpowers:add-indentation-style
Open

add indentationStyle to MarkupFormatter.Options#204
dpowers wants to merge 3 commits into
swiftlang:mainfrom
dpowers:add-indentation-style

Conversation

@dpowers

@dpowers dpowers commented Oct 10, 2024

Copy link
Copy Markdown

Summary

Add indentationStyle (.tabs/.spaces) to MarkupFormatter.Options to allow format to use tabs when outputting Markdown.

@QuietMisdreavus

Copy link
Copy Markdown
Contributor

Could you add a CLI flag to the format tool in Tools/markdown-tool/Commands/FormatCommand.swift? That way this can be set in the command-line formatter and not just the API.

@QuietMisdreavus QuietMisdreavus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good PR, but a couple more things:

  1. The indentation for content nested underneath a block directive (line 488) uses a four-space indent; i don't think we handle tabs for nested block-directive content, but if that is handled that should also be updated.
  2. Can you add some tests in MarkupFormatterTests?

Comment on lines +454 to +459
prefix += formattingOptions.indentationStyle.indentation()
}
unorderedListCount += 1
} else if element is OrderedList {
if orderedListCount > 0 {
prefix += " "
prefix += formattingOptions.indentationStyle.indentation()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These indentation markers use different numbers of spaces between ordered and unordered lists to align trailing text with the previous line. While it may still parse correctly, this is a regression in behavior.

@dpowers

dpowers commented Oct 13, 2024 via email

Copy link
Copy Markdown
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants