readline: fix issue with newline-less last line by harrisi · Pull Request #47317

readline: fix issue with newline-less last line by harrisi · Pull Request #47317 · nodejs/node

The logic for reading lines was slightly flawed, in that it assumed
there would be a final new line. It handled the case where there are no
new lines, but this then broke if there were some new lines.

The fix in logic is basically removing the special case where there are
no new lines by changing it to always read the final line with no new
lines. This works because if a file contains no new lines, the final
line is the first line, and all is well.

There is some subtlety in this functioning, however. If the last line
contains no new lines, then `lastIndex` will be the start of the last
line, and `kInsertString` will be called from that point. If it does
contain a new line, `lastIndex` will be equal to `s.length`, so the
slice will be the empty string.

Fixes: nodejs#47305

added needs-ci

PRs that need a full CI run.

readline

Issues and PRs related to the built-in readline module.

labels

Mar 30, 2023

The repl test probably isn't strictly necessary, but this area's tests
has been lacking sufficiently varying cases which have caused a few bugs
to be missed, so it seems reasonable to include.

aduh95 added author ready

PRs that have at least one approval, no pending requests for changes, and a CI started.

request-ci

Add this label to start a Jenkins CI on a PR.

labels

Apr 2, 2023

This was referenced

Apr 3, 2023

aduh95 added commit-queue

Add this label to land a pull request using GitHub Actions.

commit-queue-squash

Add this label to instruct the Commit Queue to squash all the PR commits into the first one.

labels

Apr 14, 2023

targos pushed a commit that referenced this pull request

May 2, 2023

The logic for reading lines was slightly flawed, in that it assumed
there would be a final new line. It handled the case where there are no
new lines, but this then broke if there were some new lines.

The fix in logic is basically removing the special case where there are
no new lines by changing it to always read the final line with no new
lines. This works because if a file contains no new lines, the final
line is the first line, and all is well.

There is some subtlety in this functioning, however. If the last line
contains no new lines, then `lastIndex` will be the start of the last
line, and `kInsertString` will be called from that point. If it does
contain a new line, `lastIndex` will be equal to `s.length`, so the
slice will be the empty string.

Fixes: #47305
PR-URL: #47317
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

danielleadams pushed a commit that referenced this pull request

Jul 6, 2023

The logic for reading lines was slightly flawed, in that it assumed
there would be a final new line. It handled the case where there are no
new lines, but this then broke if there were some new lines.

The fix in logic is basically removing the special case where there are
no new lines by changing it to always read the final line with no new
lines. This works because if a file contains no new lines, the final
line is the first line, and all is well.

There is some subtlety in this functioning, however. If the last line
contains no new lines, then `lastIndex` will be the start of the last
line, and `kInsertString` will be called from that point. If it does
contain a new line, `lastIndex` will be equal to `s.length`, so the
slice will be the empty string.

Fixes: #47305
PR-URL: #47317
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

MoLow pushed a commit to MoLow/node that referenced this pull request

Jul 6, 2023

The logic for reading lines was slightly flawed, in that it assumed
there would be a final new line. It handled the case where there are no
new lines, but this then broke if there were some new lines.

The fix in logic is basically removing the special case where there are
no new lines by changing it to always read the final line with no new
lines. This works because if a file contains no new lines, the final
line is the first line, and all is well.

There is some subtlety in this functioning, however. If the last line
contains no new lines, then `lastIndex` will be the start of the last
line, and `kInsertString` will be called from that point. If it does
contain a new line, `lastIndex` will be equal to `s.length`, so the
slice will be the empty string.

Fixes: nodejs#47305
PR-URL: nodejs#47317
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>