strip ansi control chars from PR comments when called with {"color": "always"}
#859
Conversation
When called with `color: always`, the PR comment includes ANSI control characters. Let's strip them using ansi-to-html
bennettp123
changed the title
{"color": "always"}
There was a problem hiding this comment.
On the whole this change LGTM. I do wonder if it makes sense to add pre tags in all cases. My two concerns are if this will affect users who have color disabled, and the performance of substringing when over max length (see inline comment).
Does GitHub Flavored Markdown always convert code blocks (```) to <pre> tags when it renders in the browser? i.e. is change equivalent when color: never?
| while (maxLength !== undefined && html.length > maxLength) { | ||
| message = message.substring(0, message.length - 1); | ||
| html = convert.toHtml(message); | ||
| } |
There was a problem hiding this comment.
Could you explain in a comment what this code is doing? It looks like it's truncating the message body according to the max length of the message.
However, it looks like it's only cutting out a single character at a time and attempting to convert to HTML again. I imagine that would be very slow if html.length >>>> maxLength.
| const summary = '<summary>Pulumi report</summary>'; | ||
|
|
||
| const rawBody = output.substring(0, 64_000); | ||
| const rawBody = ansiToHtml(output, { maxLength: 64_000 }); |
There was a problem hiding this comment.
Since this PR touches this code, would you mind pulling this value into a constant so it's not a "magic number"? I'd appreciate it :D
| const comment = comments.find( | ||
| (comment) => | ||
| comment.body.startsWith(heading) && comment.body.includes(summary), |
There was a problem hiding this comment.
This looks like its a purely aesthetic change, right? NBD, just checking.
2 participants
When
pulumi/actionsis used with{comment-on-pr: true, color: "always"}, it creates a PR comment with extra ANSI control characters.Let's strip them before adding them to the PR comment.
Uses
ansi-to-htmlto convert the output into HTML. The result is then wrapped in<pre></pre>tags before adding it to the PR comment.I have no idea if this is the best way to do it, but I thought I'd try it and see what people think. 😀
Before:
After: