Skip to content
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

Preformatting stability #78

Closed
davidtorosyan opened this issue Aug 2, 2022 · 5 comments
Closed

Preformatting stability #78

davidtorosyan opened this issue Aug 2, 2022 · 5 comments

Comments

@davidtorosyan
Copy link

Stability bug with <pre>.

Before:

/**
 * Some summary
 *
 * <pre>
 * line one
 * ```
 *     line two
 * ```
 */
fun foo() = Unit

After:

/**
 * Some summary
 *
 * ```
 *
 * line one
 *
 * ```
 *     line two
 * ```
 */
fun foo() = Unit

After again:

/**
 * Some summary
 *
 * ```
 *
 * line one
 *
 * ```
 *
 * line two
 *
 * ```
 */
fun foo() = Unit

Using v1.5.5 with default options

@tnorbye
Copy link
Owner

tnorbye commented Aug 3, 2022

The problem here is that this isn't valid markup -- the opening <pre> tag isn't closed. After recent bug fixes I tried to gracefully handle this; it isn't entirely clear what should happen, but dokka does for example interpret later @param tags as parameter documentation, so what I settled on was to "ignore" the <pre> tag.

However, the <pre> tag was still getting converted to ```. And what happens here is that it then after formatting gets paired up with the first ``` in the initial comment.

I'm fixing this such that an unmatched <pre> is not converted to ```. With that, the formatting is stable.

(An alternative would have been for an unmatched <pre> to simply apply to the rest of the comment, so nothing after <pre> gets adjusted. I'll play with this and see if it feels better. Do you have a lot of cases where people leave <pre> comments unterminated? (The Dokka formatting for that does not look good.)

@tnorbye
Copy link
Owner

tnorbye commented Aug 3, 2022

I played with it a bit and I think I'll switch it such that if you have a <pre> tag, without a closing </pre> tag, instead of just treating that <pre> line as preformatted, the entire remainder of the comment is treated as preformatted.

@tnorbye
Copy link
Owner

tnorbye commented Aug 3, 2022

(I tried inserting a final </pre> on the last line, but when I do that, a subsequent format will then turn this into ```-blocks so the formatter isn't stable. I then tried to have it directly convert the <pre> into ``` at the same time, but in a case like the above, where you already have a mixture of ``` and <pre> this made the markup even more confused. I think trying to interpret broken markup is a losing battle, which is why just treating an unterminated <pre> and leaving things alone seems like the safest course.)

@davidtorosyan
Copy link
Author

This isn't really a missing </pre> bug, it repros for this as well:

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

I'm actually fine with the final output - maybe it could be better in some cases, but I agree it's a losing battle.

What I'm more concerned with is having the formatter be idempotent. Could this be solved by doing a second pass? Or maybe by doing replacements before reflowing?

tnorbye added a commit that referenced this issue Aug 3, 2022
1.5.7 fixes the following bugs:
   - #76: Preserve newline style (CRLF on Windows)
   - #77: Preformatting error
   - #78: Preformatting stability
   - #79: Replace `{@param name}` with `[name]`
tnorbye added a commit that referenced this issue Aug 3, 2022
1.5.7 fixes the following bugs:
   - #76: Preserve newline style (CRLF on Windows)
   - #77: Preformatting error
   - #78: Preformatting stability
   - #79: Replace `{@param name}` with `[name]`
@tnorbye
Copy link
Owner

tnorbye commented Aug 3, 2022

Both of these should be handled in 1.5.7. It will treat an unterminated <pre> such that the remainder of the comment is treated as preformatted -- and when it is terminated, but doubled up with ```, it will not convert the redundant <pre> tags.

@tnorbye tnorbye closed this as completed Aug 3, 2022
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

No branches or pull requests

2 participants