readline: fix unicode line separators being ignored #57591

dario-piotrowicz · 2025-03-22T19:01:03Z

test/parallel/test-readline-line-separators.js

lpinca · 2025-03-22T19:26:36Z

I'm fine with this but it might be a breaking change.

dario-piotrowicz · 2025-03-22T19:38:23Z

I'm fine with this but it might be a breaking change.

Yes, I was thinking the same thing, but wanted to hear y'all thoughts on it 🙂
(and unfortunately until this gets merged today I think I'm too late for making it into v24 right? 😓 ... unless it gets postponed... 🤔 )

lpinca · 2025-03-22T19:39:03Z

(PS: maybe I should just not compare it to the result of the regexp splitting but add the expected array in the test instead 🤔)

Yes, I think it would remove confusion and be more readable.

(and unfortunately until this gets merged today I think I'm too late for making it into v24 right? 😓 ... unless it #57057 (comment)... 🤔 )

Perhaps, but it does not matter much, it will eventually be added to v25, it will just have to wait longer.

lpinca · 2025-03-22T19:49:08Z

I added the semver-major label as a precaution. If you think it is not necessary, feel free to remove it.

codecov · 2025-03-22T21:09:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.22%. Comparing base (86f86a2) to head (d0b4c39).
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #57591      +/-   ##
==========================================
- Coverage   90.22%   90.22%   -0.01%     
==========================================
  Files         630      630              
  Lines      185481   185488       +7     
  Branches    36364    36371       +7     
==========================================
+ Hits       167357   167361       +4     
  Misses      11004    11004              
- Partials     7120     7123       +3

Files with missing lines	Coverage Δ
lib/internal/readline/interface.js	`96.97% <100.00%> (+0.01%)`	⬆️

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lib/internal/readline/interface.js

also include \r\n in test

remove confising comparison with regexp splits

nodejs-github-bot · 2025-04-12T18:16:04Z

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

PR-URL: #57591 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: James M Snell <[email protected]>

jasnell · 2025-04-13T16:52:29Z

Landed in a42bca5

PR-URL: #57591 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: James M Snell <[email protected]>

Semver-Major Commits: assert,util: * (SEMVER-MAJOR) Revert "assert,util: revert recursive breaking change (Ruben Bridgewater) #57622 buffer: * (SEMVER-MAJOR) move SlowBuffer to EOL (James M Snell) #58008 * (SEMVER-MAJOR) make `buflen` in integer range (zhenweijin) #51821 build: * (SEMVER-MAJOR) downgrade armv7 support to experimental (Michaël Zasso) #58071 * (SEMVER-MAJOR) bump supported macOS version to 13.5 (Michaël Zasso) #57115 * (SEMVER-MAJOR) increase minimum Xcode version to 16.1 (Michaël Zasso) #56824 * (SEMVER-MAJOR) link V8 with atomic library (Michaël Zasso) #55014 * (SEMVER-MAJOR) remove support for ppc 32-bit (Michaël Zasso) #55014 * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #55014 child_process: * (SEMVER-MAJOR) deprecate passing `args` to `spawn` and `execFile` (Daniel Venable) #57199 deps: * (SEMVER-MAJOR) V8: cherry-pick f915fa4c9f41 (Olivier Flückiger) #55014 * (SEMVER-MAJOR) V8: cherry-pick 0d5d6e71bbb0 (Yagiz Nizipli) #55014 * (SEMVER-MAJOR) V8: cherry-pick 0c11feeeca4a (Michaël Zasso) #55014 * (SEMVER-MAJOR) define V8_PRESERVE_MOST as no-op on Windows (Stefan Stojanovic) #55014 * (SEMVER-MAJOR) always define V8_NODISCARD as no-op (Michaël Zasso) #55014 * (SEMVER-MAJOR) fix FP16 bitcasts.h (Stefan Stojanovic) #55014 * (SEMVER-MAJOR) patch V8 to support compilation with MSVC (StefanStojanovic) #55014 * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #55014 * (SEMVER-MAJOR) disable V8 concurrent sparkplug compilation (Michaël Zasso) #55014 * (SEMVER-MAJOR) always define V8_EXPORT_PRIVATE as no-op (Michaël Zasso) #55014 * (SEMVER-MAJOR) update V8 to 13.0.245.25 (Michaël Zasso) #55014 * (SEMVER-MAJOR) upgrade npm to 11.0.0 (npm team) #56274 * (SEMVER-MAJOR) update undici to 7.0.0 (Node.js GitHub Bot) #56070 fs: * (SEMVER-MAJOR) remove ability to call truncate with fd (Yagiz Nizipli) #57567 * (SEMVER-MAJOR) deprecate passing invalid types in `fs.existsSync` (Carlos Espa) #55753 * (SEMVER-MAJOR) runtime deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49686 * (SEMVER-MAJOR) remove `dirent.path` (Antoine du Hamel) #55548 http: * (SEMVER-MAJOR) remove outgoingmessage _headers and _headersList (Yagiz Nizipli) #57551 http2: * (SEMVER-MAJOR) session tracking and graceful server close (Kushagra Pandey) #57586 lib: * (SEMVER-MAJOR) remove obsolete Cipher export (James M Snell) #57266 * (SEMVER-MAJOR) unexpose six process bindings (Michaël Zasso) #57149 * (SEMVER-MAJOR) make ALS default to AsyncContextFrame (Stephen Belanger) #55552 * (SEMVER-MAJOR) runtime deprecate SlowBuffer (Rafael Gonzaga) #55175 net: * (SEMVER-MAJOR) make _setSimultaneousAccepts() end-of-life deprecated (Yagiz Nizipli) #57550 readline: * (SEMVER-MAJOR) add stricter validation for functions called after closed (Dario Piotrowicz) #57680 * (SEMVER-MAJOR) fix unicode line separators being ignored (Dario Piotrowicz) #57591 repl: * (SEMVER-MAJOR) runtime deprecate instantiating without new (Aviv Keller) #54869 src: * (SEMVER-MAJOR) update GetForegroundTaskRunner override (Etienne Pierre-doray) #55014 * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 134 (Michaël Zasso) #55014 * (SEMVER-MAJOR) drop --experimental-permission in favour of --permission (Rafael Gonzaga) #56240 * (SEMVER-MAJOR) add async context frame to AsyncResource (Gerhard Stöbich) #56082 * (SEMVER-MAJOR) nuke deprecated and un-used enum members in `OptionEnvvarSettings` (Juan José) #53079 stream: * (SEMVER-MAJOR) catch and forward error from dest.write (jakecastelli) #55270 test: * (SEMVER-MAJOR) disable fast API call count checks (Michaël Zasso) #55014 test_runner: * (SEMVER-MAJOR) remove promises returned by t.test() (Colin Ihrig) #56664 * (SEMVER-MAJOR) remove promises returned by test() (Colin Ihrig) #56664 * (SEMVER-MAJOR) automatically wait for subtests to finish (Colin Ihrig) #56664 timers: * (SEMVER-MAJOR) check for immediate instance in clearImmediate (Gürgün Dayıoğlu) #57069 * (SEMVER-MAJOR) set several methods EOL (Yagiz Nizipli) #56966 tls: * (SEMVER-MAJOR) remove deprecated tls.createSecurePair (Jonas) #57361 * (SEMVER-MAJOR) make server.prototype.setOptions end-of-life (Yagiz Nizipli) #57339 tools: * (SEMVER-MAJOR) update V8 gypfiles for 13.0 (Michaël Zasso) #55014 url: * (SEMVER-MAJOR) expose urlpattern as global (Jonas) #56950 * (SEMVER-MAJOR) runtime deprecate url.parse (Yagiz Nizipli) #55017 zlib: * (SEMVER-MAJOR) deprecate classes usage without `new` (Yagiz Nizipli) #55718 PR-URL: #57609 Signed-off-by: RafaelGSS <[email protected]>

PR-URL: #57591 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jordan Harband <[email protected]> Reviewed-By: James M Snell <[email protected]>

Semver-Major Commits: assert,util: * (SEMVER-MAJOR) Revert "assert,util: revert recursive breaking change (Ruben Bridgewater) #57622 buffer: * (SEMVER-MAJOR) move SlowBuffer to EOL (James M Snell) #58008 * (SEMVER-MAJOR) make `buflen` in integer range (zhenweijin) #51821 build: * (SEMVER-MAJOR) update list of installed cppgc headers (Michaël Zasso) #58070 * (SEMVER-MAJOR) fix V8 TLS config for shared lib builds (Michaël Zasso) #58070 * (SEMVER-MAJOR) pass `-fPIC` to linker as well for shared builds (Michaël Zasso) #58070 * (SEMVER-MAJOR) add `/bigobj` to compile V8 on Windows (Michaël Zasso) #58070 * (SEMVER-MAJOR) enable shared RO heap with ptr compression (Michaël Zasso) #58070 * (SEMVER-MAJOR) remove support for s390 32-bit (Richard Lau) #58070 * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #58070 * (SEMVER-MAJOR) downgrade armv7 support to experimental (Michaël Zasso) #58071 * (SEMVER-MAJOR) bump supported macOS version to 13.5 (Michaël Zasso) #57115 * (SEMVER-MAJOR) increase minimum Xcode version to 16.1 (Michaël Zasso) #56824 * (SEMVER-MAJOR) link V8 with atomic library (Michaël Zasso) #55014 * (SEMVER-MAJOR) remove support for ppc 32-bit (Michaël Zasso) #55014 * (SEMVER-MAJOR) reset embedder string to "-node.0" (Michaël Zasso) #55014 build,src,tools: * (SEMVER-MAJOR) adapt build config for V8 13.3 (Michaël Zasso) #58070 child_process: * (SEMVER-MAJOR) deprecate passing `args` to `spawn` and `execFile` (Daniel Venable) #57199 deps: * (SEMVER-MAJOR) remove deps/simdutf (Michaël Zasso) #58070 * (SEMVER-MAJOR) V8: backport 954187bb1b87 (Joyee Cheung) #58070 * (SEMVER-MAJOR) patch V8 to support compilation with MSVC (StefanStojanovic) #58070 * (SEMVER-MAJOR) always define V8_EXPORT_PRIVATE as no-op (Michaël Zasso) #58070 * (SEMVER-MAJOR) disable V8 concurrent sparkplug compilation (Michaël Zasso) #58070 * (SEMVER-MAJOR) use std::map in MSVC STL for EphemeronRememberedSet (Joyee Cheung) #58070 * (SEMVER-MAJOR) patch V8 for illumos (Dan McDonald) #58070 * (SEMVER-MAJOR) remove problematic comment from v8-internal (Michaël Zasso) #58070 * (SEMVER-MAJOR) define V8_PRESERVE_MOST as no-op on Windows (Stefan Stojanovic) #58070 * (SEMVER-MAJOR) fix FP16 bitcasts.h (Stefan Stojanovic) #58070 * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #58070 * (SEMVER-MAJOR) update V8 to 13.6.233.8 (Michaël Zasso) #58070 * (SEMVER-MAJOR) V8: cherry-pick f915fa4c9f41 (Olivier Flückiger) #55014 * (SEMVER-MAJOR) V8: cherry-pick 0d5d6e71bbb0 (Yagiz Nizipli) #55014 * (SEMVER-MAJOR) V8: cherry-pick 0c11feeeca4a (Michaël Zasso) #55014 * (SEMVER-MAJOR) define V8_PRESERVE_MOST as no-op on Windows (Stefan Stojanovic) #55014 * (SEMVER-MAJOR) always define V8_NODISCARD as no-op (Michaël Zasso) #55014 * (SEMVER-MAJOR) fix FP16 bitcasts.h (Stefan Stojanovic) #55014 * (SEMVER-MAJOR) patch V8 to support compilation with MSVC (StefanStojanovic) #55014 * (SEMVER-MAJOR) patch V8 to avoid duplicated zlib symbol (Michaël Zasso) #55014 * (SEMVER-MAJOR) disable V8 concurrent sparkplug compilation (Michaël Zasso) #55014 * (SEMVER-MAJOR) always define V8_EXPORT_PRIVATE as no-op (Michaël Zasso) #55014 * (SEMVER-MAJOR) update V8 to 13.0.245.25 (Michaël Zasso) #55014 * (SEMVER-MAJOR) upgrade npm to 11.0.0 (npm team) #56274 * (SEMVER-MAJOR) update undici to 7.0.0 (Node.js GitHub Bot) #56070 fs: * (SEMVER-MAJOR) remove ability to call truncate with fd (Yagiz Nizipli) #57567 * (SEMVER-MAJOR) deprecate passing invalid types in `fs.existsSync` (Carlos Espa) #55753 * (SEMVER-MAJOR) runtime deprecate `fs.F_OK`, `fs.R_OK`, `fs.W_OK`, `fs.X_OK` (Livia Medeiros) #49686 * (SEMVER-MAJOR) remove `dirent.path` (Antoine du Hamel) #55548 http: * (SEMVER-MAJOR) remove outgoingmessage _headers and _headersList (Yagiz Nizipli) #57551 http2: * (SEMVER-MAJOR) session tracking and graceful server close (Kushagra Pandey) #57586 lib: * (SEMVER-MAJOR) remove obsolete Cipher export (James M Snell) #57266 * (SEMVER-MAJOR) unexpose six process bindings (Michaël Zasso) #57149 * (SEMVER-MAJOR) make ALS default to AsyncContextFrame (Stephen Belanger) #55552 * (SEMVER-MAJOR) runtime deprecate SlowBuffer (Rafael Gonzaga) #55175 net: * (SEMVER-MAJOR) make _setSimultaneousAccepts() end-of-life deprecated (Yagiz Nizipli) #57550 readline: * (SEMVER-MAJOR) add stricter validation for functions called after closed (Dario Piotrowicz) #57680 * (SEMVER-MAJOR) fix unicode line separators being ignored (Dario Piotrowicz) #57591 repl: * (SEMVER-MAJOR) runtime deprecate instantiating without new (Aviv Keller) #54869 src: * (SEMVER-MAJOR) use non-deprecated WriteUtf8V2() method (Yagiz Nizipli) #58070 * (SEMVER-MAJOR) use non-deprecated Utf8LengthV2() method (Yagiz Nizipli) #58070 * (SEMVER-MAJOR) use V8-owned CppHeap (Joyee Cheung) #58070 * (SEMVER-MAJOR) use `v8::ExternalMemoryAccounter` (Michaël Zasso) #58070 * (SEMVER-MAJOR) replace uses of FastApiTypedArray (Michaël Zasso) #58070 * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 137 (Michaël Zasso) #58070 * (SEMVER-MAJOR) update GetForegroundTaskRunner override (Etienne Pierre-doray) #55014 * (SEMVER-MAJOR) update NODE_MODULE_VERSION to 134 (Michaël Zasso) #55014 * (SEMVER-MAJOR) drop --experimental-permission in favour of --permission (Rafael Gonzaga) #56240 * (SEMVER-MAJOR) add async context frame to AsyncResource (Gerhard Stöbich) #56082 * (SEMVER-MAJOR) nuke deprecated and un-used enum members in `OptionEnvvarSettings` (Juan José) #53079 src,test: * (SEMVER-MAJOR) unregister the isolate after disposal and before freeing (Joyee Cheung) #58070 * (SEMVER-MAJOR) add V8 API to test the hash seed (Michaël Zasso) #58070 stream: * (SEMVER-MAJOR) catch and forward error from dest.write (jakecastelli) #55270 test: * (SEMVER-MAJOR) fix test-fs-write for V8 13.6 (Michaël Zasso) #58070 * (SEMVER-MAJOR) handle explicit resource management globals (Michaël Zasso) #58070 * (SEMVER-MAJOR) adapt assert tests to stack trace changes (Michaël Zasso) #58070 * (SEMVER-MAJOR) update test-linux-perf-logger (Michaël Zasso) #58070 * (SEMVER-MAJOR) Revert "test: disable fast API call count checks (Michaël Zasso) #58070 * (SEMVER-MAJOR) disable fast API call count checks (Michaël Zasso) #55014 test_runner: * (SEMVER-MAJOR) remove promises returned by t.test() (Colin Ihrig) #56664 * (SEMVER-MAJOR) remove promises returned by test() (Colin Ihrig) #56664 * (SEMVER-MAJOR) automatically wait for subtests to finish (Colin Ihrig) #56664 timers: * (SEMVER-MAJOR) check for immediate instance in clearImmediate (Gürgün Dayıoğlu) #57069 * (SEMVER-MAJOR) set several methods EOL (Yagiz Nizipli) #56966 tls: * (SEMVER-MAJOR) remove deprecated tls.createSecurePair (Jonas) #57361 * (SEMVER-MAJOR) make server.prototype.setOptions end-of-life (Yagiz Nizipli) #57339 tools: * (SEMVER-MAJOR) update V8 gypfiles for 13.6 (Michaël Zasso) #58070 * (SEMVER-MAJOR) update V8 gypfiles for 13.5 (Michaël Zasso) #58070 * (SEMVER-MAJOR) update license-builder and LICENSE for V8 deps (Michaël Zasso) #58070 * (SEMVER-MAJOR) update V8 gypfiles for 13.4 (Michaël Zasso) #58070 * (SEMVER-MAJOR) update V8 gypfiles for 13.2 (Michaël Zasso) #58070 * (SEMVER-MAJOR) update V8 gypfiles for 13.1 (Michaël Zasso) #58070 * (SEMVER-MAJOR) update V8 gypfiles for 13.0 (Michaël Zasso) #55014 url: * (SEMVER-MAJOR) expose urlpattern as global (Jonas) #56950 * (SEMVER-MAJOR) runtime deprecate url.parse (Yagiz Nizipli) #55017 zlib: * (SEMVER-MAJOR) deprecate classes usage without `new` (Yagiz Nizipli) #55718 PR-URL: #57609 Signed-off-by: RafaelGSS <[email protected]>

nodejs-github-bot added needs-ci PRs that need a full CI run. readline Issues and PRs related to the built-in readline module. labels Mar 22, 2025

lpinca reviewed Mar 22, 2025

View reviewed changes

test/parallel/test-readline-line-separators.js Outdated Show resolved Hide resolved

dario-piotrowicz requested a review from lpinca March 22, 2025 19:38

lpinca approved these changes Mar 22, 2025

View reviewed changes

lpinca added the semver-major PRs that contain breaking changes and should be released in the next major version. label Mar 22, 2025

anonrig approved these changes Mar 23, 2025

View reviewed changes

anonrig added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Mar 24, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 1, 2025

This comment was marked as outdated.

Sign in to view

ljharb reviewed Apr 2, 2025

View reviewed changes

lib/internal/readline/interface.js Show resolved Hide resolved

ljharb approved these changes Apr 2, 2025

View reviewed changes

jasnell approved these changes Apr 5, 2025

View reviewed changes

This comment was marked as outdated.

Sign in to view

dario-piotrowicz added 3 commits April 12, 2025 16:43

readline: fix unicode line separators being ignored

5223706

fixup! readline: fix unicode line separators being ignored

a5b3e45

also include \r\n in test

fixup! readline: fix unicode line separators being ignored

d0b4c39

remove confising comparison with regexp splits

dario-piotrowicz force-pushed the dario/22448/readline-unicode-line-separators branch from 9619d97 to d0b4c39 Compare April 12, 2025 15:44

jasnell approved these changes Apr 12, 2025

View reviewed changes

jasnell closed this Apr 13, 2025

dario-piotrowicz deleted the dario/22448/readline-unicode-line-separators branch April 13, 2025 17:19

RafaelGSS mentioned this pull request May 1, 2025

2025-05-06, Version 24.0.0 (Current) #57609

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

readline: fix unicode line separators being ignored #57591

readline: fix unicode line separators being ignored #57591

dario-piotrowicz commented Mar 22, 2025

lpinca commented Mar 22, 2025

dario-piotrowicz commented Mar 22, 2025 •

edited

Loading

lpinca commented Mar 22, 2025 •

edited

Loading

lpinca commented Mar 22, 2025

codecov bot commented Mar 22, 2025 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Apr 12, 2025 •

edited by jasnell

Loading

jasnell commented Apr 13, 2025

readline: fix unicode line separators being ignored #57591

readline: fix unicode line separators being ignored #57591

Conversation

dario-piotrowicz commented Mar 22, 2025

lpinca commented Mar 22, 2025

dario-piotrowicz commented Mar 22, 2025 • edited Loading

lpinca commented Mar 22, 2025 • edited Loading

lpinca commented Mar 22, 2025

codecov bot commented Mar 22, 2025 • edited Loading

Codecov Report

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

nodejs-github-bot commented Apr 12, 2025 • edited by jasnell Loading

jasnell commented Apr 13, 2025

dario-piotrowicz commented Mar 22, 2025 •

edited

Loading

lpinca commented Mar 22, 2025 •

edited

Loading

codecov bot commented Mar 22, 2025 •

edited

Loading

nodejs-github-bot commented Apr 12, 2025 •

edited by jasnell

Loading