MRVA: Generate markdown for raw result tables#1323
Conversation
| break; | ||
| } | ||
| // `|` characters break the table, so we need to escape them | ||
| return cellValue.replace('|', '\\|'); |
Check failure
Code scanning / CodeQL
Incomplete string escaping or encoding
There was a problem hiding this comment.
Ah...thanks code scanning! I think you want to use replaceAll here.
There was a problem hiding this comment.
Can you add a line to one of the tests where there are multiple | in the same table row?
There was a problem hiding this comment.
Ugh, I broke things more 😅 Will come back to this tomorrow!
There was a problem hiding this comment.
replaceAll was introduced in node 15. And the package.json requires node >=14.17.1. So, I'm guessing that your local node version is too old.
Either:
- We change the requirement to node 15 or greater (double check exactly which version the function was added)
- Do multi-replacement in a different way. It should work if you call
replacewith a regex and use aglobaltag. eg-/\\|/g(this regex may not be right...., but it's something like that)
There was a problem hiding this comment.
You were right about needing to upgrade! (package.json updated in #1326)
I'm still slightly confused why replaceAll worked fine in other places like here: https://github.com/github/vscode-codeql/blob/main/extensions/ql-vscode/src/remote-queries/view/FileCodeSnippet.tsx#L24 😅
There was a problem hiding this comment.
At runtime, vscode is currently using node v16.13.0, so we can use it in the web view and in production. It's just that we could't use replaceAll in tests since our test version of node was too old. And we are not running any tests on our web view. 😢
|
Give me a shout if you need any help getting things green! |

Follow-up to #1318 for rendering raw results! This PR adds the actual table with results (see internal linked issue).
Some example rendered results here and here 🖼️ 🎨
(It looks kinda ugly, but maybe that's mainly because these example results and column names are fairly meaningless 😂)
Checklist
N/A - still internal only 🕳️
ready-for-doc-reviewlabel there.