FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Feature/#23 handle carriage returns by X1Vi · Pull Request #325 · extrabacon/python-shell · GitHub

Repository navigation

Feature/#23 handle carriage returns - #325

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

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

Conversation

X1Vi commented May 28, 2026 •
edited
Loading

Copy link
Copy Markdown

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 commented May 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

@extrabacon Are you reviewing this ?

StantonMatt left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL