CC @nodejs/repl

Could you add some tests ?

Tests have been added @ZYSzys, sorry I forgot to let you know when I pushed the commits.

The failed tests appear unrelated.

(got review - removing label)

@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

I'm not sure if this is semver-minor / notable-change

I think both :)

The notable-change PRs with changes that should be highlighted in changelogs. 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.

@MoLow ive added both labels.

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/highlight module, 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

CI: https://ci.nodejs.org/job/node-test-pull-request/60923/

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.

Fixed the issue, readded the label.

The failed CI is unrelated.

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?

I no longer think this is a good idea. See the comment above.

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.