fix(pr): fix broken markdown when version contains '|'

[djcsdy]

Fixes broken markdown in PRs caused by version ranges containing |.

For an example of what I mean, see this PR: softwareventures/tslint-rules#25

Notice that the table markup is broken because the target version range is ^1.9.3 || ^2.0.0, and | is interpreted by markdown as a table cell delimiter.

Closes #5397

[rarkins]

We need to check how this impacts the various platforms we support. If they handle this differently then we may need to move this massaging logic into each platform's markdown massage logic.

[lgtm-com]

This pull request introduces 1 alert when merging 92ce4aa into 61c4fcd - view on LGTM.com

new alerts:

• 1 for Incomplete string escaping or encoding

[djcsdy]

@rarkins OK. Fair point. I took a look at some of the platforms I know to be supported by Renovate.

GitHub: This change should work as-is. Using <code> instead of backtick and · instead of | also works, and is more compatible with CommonMark so might be more future-proof.

Gitlab: Exactly like GitHub.

BitBucket: It’s impossible to fix this problem, because BitBucket’s flavour of markdown doesn’t support escaping |. It’s not possible to fall back to HTML either because, although BitBucket’s documentation says that “a restricted set of HTML tags” is supported, my testing suggests that the restricted set they’re referring to is the empty set.

Gitea: no change is required, the formatting worked as it was.

CommonMark (0.29): my reading of the spec is that Gitea’s behaviour is correct. No surprises here: the open source project matches the spec, Atlassian’s implementation is, as usual, terrible, and the others are somewhere in between.

So... I guess this could be considered a bug in GitHub, Gitlab, and BitBucket, and not in Renovate. It’s still really annoying though.

I see that there is some platform-specific code in the platform directory. If you’d welcome platform-specific fixes for this problem I’d be happy to oblige. Let me know if you have any suggestions.

[viceice]

See here: #3799 (comment)
I've tested almost all our supported platforms

[rarkins]

Can we fix this using our getPrBody() functions for each platform? They are a bit misnamed these days - should maybe be massagedMarkdown() or something similar

[viceice]

Should be doable, I think bitbucket server is now fixed too, so we can use the escaped variant there too.

[rarkins]

So we default to the "correct" way (that gitea supports) and then massage it to escaped on other platforms?

[viceice]

@djcsdy @rarkins I want to summarize what needs to be done here.

• All platforms (excerpt gittea) are render the pipes wrong (current state).
• with this change only bitbucket cloud will render the pipes wrong (as before).
• this change is not nessessary for gittea but works there too.

So what shpould we do here? replacing it in getPrBody of platfrom can be difficult, can be replace additional places where not appropriate.

[rarkins]

@viceice let's leave bitbucket cloud broken for now then, so that we can move forward with the others.

[rarkins]

Please add a test or two though

[rarkins]

@djcsdy @viceice I'm a bit confused as to the status of this. Does this PR still fix things?

[viceice]

Yes, escaping is more standard and fixes bitbucket server. Bitbucket cloud is broken with both. Other platforms work before and after.

[rarkins]

From a virus code review, I can't work out what difference this PR makes

[viceice]

It escapes the pipe in version ranges. See linked or for broken GitHub PR. So this will fix GitHub pr's too

[viceice]

I try to add a test PR for current state tomorrow, so we can see this pr failing and fix text afterwards.

[rarkins]

@viceice I'm confused because I don't notice any change to the escaping:

[djcsdy]

The important part of this PR was this line:

https://github.com/renovatebot/renovate/pull/6235/files#diff-464116b52af741627d80e2939c215721L65

But someone has already implemented the equivalent change in master. So this PR is redundant now.

[viceice]

already fixed in #6514