nodejs
repl: support syntax highlighting
Instead of sending many REPL changes in one PR (#52965), it was recommended to me to implement these one-by-one in several PRs.
First up: Syntax highlighting :tada:
This PR adds RegEx-based syntax highlighting to the REPL.
Notable change text (if text this short is even worth it): "The REPL now includes syntax highlighting for input."
Tests have been added @ZYSzys, sorry I forgot to let you know when I pushed the commits.
The failed tests appear unrelated.
@atlowChemi
All review conversations have been resolved, any more notes?
@RedYetiDev do you have some screenshots or a short video of the usage? I am not certain in what way the syntax highlighting has changed :)
@BridgeAR highlighting is done to input, and it follows (mostly) the same rules as util.inspect.
New
Old
I'm not sure if this is semver-minor / notable-change
The https://github.com/nodejs/node/labels/notable-change label has been added by @RedYetiDev.
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
All checks have passed, but I assume this needs a CI to land. If someone could review, approve, and CI, it would be appreciated. Thanks :-)!
The PR currently only tests the new
internal/repl/highlightmodule, but I think tests should be aded to ensure the input is colored in REPL as well.
The highlight function is called directly within the REPL writing, so I'm not sure how to hook into it's input writing, but I believe the tests currently in use suffice.
so I'm not sure how to hook into it's output
I'd look at test like test-repl-autocomplete.js, test-repl-context.js, and test-repl-editor.js etc
I'm a little concerned about the failure at https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_withoutssl_x64/45088/testReport/junit/(root)/parallel/test_repl_preview_without_inspector/, but it didn't seem to fail anywhere else 🤔
I've removed author ready, because I don't think this should land until that is resolved. Feel free to re add the label if you disagree.
https://ci.nodejs.org/job/node-test-commit-linux-containered/45732/ is failing, and that concerns me that this API is adding to many control sequences, and it'll clutter developers trying to simply parse the output. I'm wondering if this should be closed for now, until a time where it can be implemented with less control sequencnes?
and it'll clutter developers trying to simply parse the output
If that would be an issue, the NO_COLOR variable can be set etc, or we could add a util such as unstyleText.
I don't think this is a real issue, though this might be a semver-major
If you say so, I trust your opinion. I'll keep this open in that case, though I have my reservations.
Hey y'all, I don't have the time to persue this feature, given the other Node.js-related things I am working on. I'm closing this PR, but I'm happy for someone to continue the work.