Skip to content

Turn on object mode for the readable side#14

Merged
mourner merged 1 commit intomasterfrom
object-mode
Dec 3, 2023
Merged

Turn on object mode for the readable side#14
mourner merged 1 commit intomasterfrom
object-mode

Conversation

@mourner
Copy link
Collaborator

@mourner mourner commented Oct 23, 2018

Fixes #12 (better late than never!). Also has a nice side effect of ~8% performance improvement.

I assume this would warrant a major version bump just to be on the safe side?

@mourner
Copy link
Collaborator Author

mourner commented Oct 23, 2018

@cc @tyrasd @myndzi thoughts?

@myndzi
Copy link

myndzi commented Oct 23, 2018

Seems to do the thing. I still have a difference in behavior with my tests regarding empty tokens and empty delimiters primarily; for example, with the input "abcde" and a delimiter of "", I emit ["a", "b", "c", "d", "e"], while this package seems to emit "abcde". This could be seen as a behavioral choice and not a bug, but diverges from String.prototype.split. Equally, my code emits empty values in the case of an input such as "abba" with a delimiter of "b": ["a", "", "a"], while this code emits ["a", "a"]. Same thing. Neither really affects this PR, but they do block the PR I was originally preparing when filing #12 (which was to integrate my test suite into this package).

@myndzi
Copy link

myndzi commented Oct 23, 2018

(If you're interested, the WIP branch is here: https://github.com/myndzi/binary-split/tree/integrate-new-tests -- I didn't bother to convert the test runner yet since the tests simply fail)

@mourner
Copy link
Collaborator Author

mourner commented Oct 24, 2018

@myndzi good points! I think we should follow the same logic for edge cases as split and split2 packages — let's check how those behave.

@mourner
Copy link
Collaborator Author

mourner commented Oct 24, 2018

@myndzi results for split2 are mixed:

  • '.a..b.' / '.' -> ['', 'a', '', 'b']
  • 'abcd' / '' -> ['abcd']

@mourner mourner merged commit 15dd880 into master Dec 3, 2023
@mourner mourner deleted the object-mode branch December 3, 2023 13:01
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.

Streams2 interface not splitting correctly?

2 participants