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

renovate-bot open a PR with wrong Markdown table rendering result #5397

Closed
ocavue opened this issue Feb 6, 2020 · 3 comments
Closed

renovate-bot open a PR with wrong Markdown table rendering result #5397

ocavue opened this issue Feb 6, 2020 · 3 comments
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality

Comments

@ocavue
Copy link
Contributor

ocavue commented Feb 6, 2020

What Renovate type are you using?

Hosted GitHub App.

Describe the bug

I use "rangeStrategy": "widen" in my renovate configuration. So Renovate will upgrade my dependency from ^24.0.0 to ^24.0.0 || ^25.0.0.

In the PR Renovate generated for this upgrade, Renovate wrote something like below in the PR description:

| Package | Type | Update | Change |
|---|---|---|---|
| jest | dependencies | major | [`^24.0.0` -> `^24.0.0 || ^25.0.0`](https://url.io/) |

This table will be rendered as the HTML below:

Package Type Update Change
jest dependencies major [^24.0.0 -> `^24.0.0

image

The || ^25.0.0 part is gone because of the Markdown table syntax.

Did you see anything helpful in debug logs?

N/A

To Reproduce

ocavue/jest-puppeteer-istanbul#17 (comment)

Additional context

This rendering error can be fixed by adding a \ before |.

| Package | Type | Update | Change |
|---|---|---|---|
| jest | dependencies | major | [`^24.0.0` -> `^24.0.0 \|\| ^25.0.0`](https://url.io/) |
Package Type Update Change
jest dependencies major ^24.0.0 -> ^24.0.0 || ^25.0.0
@rarkins rarkins added type:bug Bug fix of existing functionality priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others ready labels Feb 6, 2020
@rarkins
Copy link
Collaborator

rarkins commented Feb 6, 2020

I wonder if this is as simple as escaping all | symbols within table contents, or if that itself would introduce any undesirable outcomes.

@ocavue
Copy link
Contributor Author

ocavue commented Feb 6, 2020

If we use HTML instead of Markdown, we don't need escape | symbols.

<table>
    <thead>
        <tr>
            <th>Package</th>
            <th>Type</th>
            <th>Update</th>
            <th>Change</th>
        </tr>
    </thead>
    <tbody>
        <tr>
            <td>jest</td>
            <td>dependencies</td>
            <td>major</td>
            <td>
                <a href="https://url.io/" rel="nofollow">
                    <code>^24.0.0</code> -&gt; <code>^24.0.0 || ^25.0.0</code>
                </a>
            </td>
        </tr>
    </tbody>
</table>
Package Type Update Change
jest dependencies major ^24.0.0 -> ^24.0.0 || ^25.0.0

However, we have to escape > symbol in this example 😂. So there is always something to escape.

Based on GitHub Flavored Markdown Spec, there seems no other way.

@renovatebot renovatebot deleted a comment from ocavue Feb 6, 2020
@rarkins
Copy link
Collaborator

rarkins commented Feb 7, 2020

Thanks for checking that out. I prefer we stick with markdown for now anyway (more cross-platform compatible) so we'll work on escaping the ||.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:bug Bug fix of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants