tnorbye / kdoc-formatter

Reformats Kotlin KDoc comments, reflowing text and other cleanup, both via IDE plugin and command line utility
Apache License 2.0
73 stars 2 forks source link

Preformatting error #77

Closed davidtorosyan closed 2 years ago

davidtorosyan commented 2 years ago

Some strings break preformatting (note that {@ is the minimal string I found, but you can add pre and postfixes and still repro).

Before:

/**
 * Some summary.
 *
 * {@
 * ```
 *    Some code.
 * ```
 */
fun foo() = Unit

After:

/**
 * Some summary.
 *
 * {@ ``` Some code. ```
 */
fun foo() = Unit

Using v1.5.5 with default options

tnorbye commented 2 years ago

That looks to me like incorrect markup. Do you know what specific tag was there? Was it {@code} ? That's the only tag I can imagine where it makes sense that you'd have multiple lines inside { } -- but on the other hand, Dokka does not seem to support this.

It's difficult to know how to handle invalid markup. Right now the formatter will take text inside a {@-} block and flow it as normal paragraph text. I don't think Dokka supports anything else, but I guess it could try to recognize special patterns where it looks like the comment is trying to do something else (such as the above case, and maybe blank lines) and in that case leave the remainder of the comment (or until the } end) alone.

davidtorosyan commented 2 years ago

Do you know what specific tag was there? Was it {@code}?

You guessed it. The original looked closer to this, which I admit is very weird (why is it triple formatted?):

/**
 * Some summary
 *
 * { <pre> {@code
 * ```
 *    Some code
 * ```
 * }<pre/>
 */
fun foo() = Unit

It's difficult to know how to handle invalid markup

Isn't something like <b> invalid markup as well? I figured the approach would be either (a) convert to the nearest Dokka equivalent, (b) remove it, (c) treat is as normal text.

Right now the formatter will take text inside a {@-} block and flow it as normal paragraph text

Do you know why it does that? Is {@ something meaningful?

tnorbye commented 2 years ago

I'd be afraid of just removing it, in case it was intended as meaningful text, not just markup (e.g. the text "from to " might not render well at all but somebody reading the comment in the future may get something out of it; if it just silently disappeared that wouldn't be great).

The "{@ ...}" construct is the javadoc construct, generally supported by kdoc -- e.g. {@link foo}, {@linkplain some link}, {@code some code snippet}, {@param some reference}, etc. For all the cases I can find, the marked up text inside the { } is textual. Making sure the text is not interpreted in some other way was in fact a part of a bug fix last week -- if you have the text {@link #foo}, and then that "#foo" is placed at the beginning of a new line, Dokka would take the "#" as a paragraph heading.)

davidtorosyan commented 2 years ago

For something like {@link foo}, I understand treating it differently since you can convert it to [foo].

For multiline {@code you could treat it as triple-backtick, and for single-line {@code foo} you could fo [foo] (though you're not doing it now, which is also fine).

For unhandled sequences like {@random I'd just treat it as a string literal instead of trying to flow it into a paragraph. The main concern I have is that the valid backticks in this case end up not getting respected. Thoughts?

tnorbye commented 2 years ago

In 1.5.7 this should be a bit more conservative -- if it notices markup like ``` or blank lines inside a {@} span it will leave it alone.

I also made the tag conversion a bit more conservative: if a <pre> block also contains ``` markers, it will leave the <pre> tags alone.