connor4312 / nodejs-testing

VS Code integration for node:test native tests
MIT License
43 stars 6 forks source link

feat: Handle ANSI codes in test failure message #29

Closed rluvaton closed 4 months ago

rluvaton commented 4 months ago

Fixes #27

Todo

Before:

image

After:

image
rluvaton commented 4 months ago

Fixed conflicts

connor4312 commented 4 months ago

Thanks! The loss of monospace is unfortunate, I wonder if we can bring it back in markdown, maybe by wrapping the contents in a pre or code tag?

rluvaton commented 4 months ago

Regarding the trusted HTML and escaping, looking at vscode source code it handle the sanitize and escaping and protect from XSS

https://github.com/microsoft/vscode/blob/6d2920473c6f13759c978dd89104c4270a83422d/src/vs/base/browser/markdownRenderer.ts

connor4312 commented 4 months ago

Yea, but the test output could still include HTML (e.g. tests for a library that does anything with HTML) which should be rendered as text and not interpreted as HTML

rluvaton commented 4 months ago

Thanks, will add test but making the output with monospace worked + using the css variables, thanks

also, great catch with the HTML that should be escaped

fixed the comments but still need to add tests, updated the image in the pr description to show the updated output

connor4312 commented 4 months ago

Hi, thanks again for the PR.

I actually got another request for this on VS Code. Since this is a fairly common problem and the solution is (as demonstrated) kind of hard to get right for extensions, I have made a change in VS Code to respect and colorize ANSI sequences automatically https://github.com/microsoft/vscode/pull/207852

Again, I really appreciate you taking the time to contribute this! I'm sorry it didn't end up getting this way.

rluvaton commented 4 months ago

Thank you! It's even better