Skip to content

Conversation

sebastienros
Copy link
Owner

Fixes #219

@lahma I updated the characters mask manually, are you mad?

/cc @gpetrou

@sebastienros sebastienros requested a review from lahma June 6, 2025 18:45
Copy link
Collaborator

@lahma lahma left a comment

Choose a reason for hiding this comment

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

Looks good to me 😄

@@ -105,6 +105,7 @@ public static bool IsWhiteSpaceOrNewLine(char ch)
return (ch <= 32 &&
((ch == 32) || // space
(ch == '\n') ||
(ch == '\f') ||
(ch == '\r') ||
(ch == '\t') || // horizontal tab
(ch == '\v')))
Copy link

Choose a reason for hiding this comment

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

@sebastienros thanks for fixing this.
Since these are consecutive now, is it worth checking the range instead? I mean something like ch >= 0x09 && ch <= 0x0d

0x09 9 HT Horizontal Tab
0x0a 10 LF Line Feed
0x0b 11 VT Vertical Tab
0x0c 12 FF Form Feed
0x0d 13 CR Carriage Return

Copy link
Owner Author

Choose a reason for hiding this comment

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

Since this is a test it doesn't matter to me. On the contrary even since what it's checking it more obvious (again at least to me).

@sebastienros sebastienros merged commit 9265bce into main Jun 10, 2025
1 check passed
@sebastienros sebastienros deleted the sebros/consumenumber branch June 10, 2025 17:54
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.

Using form feed as separator
3 participants