Skip to content

Feature/#23 handle carriage returns#325

Open
X1Vi wants to merge 2 commits into
extrabacon:masterfrom
X1Vi:feature/#23-handle-carriage-returns
Open

Feature/#23 handle carriage returns#325
X1Vi wants to merge 2 commits into
extrabacon:masterfrom
X1Vi:feature/#23-handle-carriage-returns

Conversation

@X1Vi
Copy link
Copy Markdown

@X1Vi X1Vi commented May 28, 2026

Fixes #23
I have added CR Pattern handling.
We are using a boolean to enable this functionality and check for Window, Linux and MacOS.

If False then we execute then normal flow instead of the True flow

I am assuming \n is for linux and macOS, \r\n is for windows and \r is for crdata.

I have added some tests for this as well that checks basic functionality.

I ran npm test and all test were passing

@X1Vi
Copy link
Copy Markdown
Author

X1Vi commented May 29, 2026

@extrabacon Are you reviewing this ?

Copy link
Copy Markdown

@StantonMatt StantonMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I checked this locally and npm test passes for me (45 passing).

One edge case seems worth covering before merge: when handleCarriageReturn is enabled and a normal Windows \r\n newline is split across stream chunks, NewlineTransformer currently treats the trailing \r as a standalone carriage-return update. That means the line is emitted as crdata instead of message.

I reproduced it directly against this branch:

const { NewlineTransformer } = require("./index.ts");
const transformer = new NewlineTransformer(true);
const data = [];
const cr = [];
transformer.on("data", chunk => data.push(String(chunk)));
transformer.on("cr", chunk => cr.push(String(chunk)));
transformer.write("first\r");
transformer.write("\nsecond\n");
transformer.end();
transformer.on("end", () => console.log({ data, cr }));

Current output is effectively:

{ data: ["second"], cr: ["first"] }

For a split \r\n, I would expect first to remain a normal newline-delimited message, not a crdata update. A focused transformer test for that chunk-boundary case would lock this down.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[New Feature] Gracefully handle carriage returns

2 participants