Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: slice considers intro/outro, no throw after removal #62

Merged
merged 1 commit into from
May 14, 2016

Conversation

leebyron
Copy link
Contributor

@leebyron leebyron commented May 11, 2016

This fixes a few issues with slice introduced recently. Tests are added which fail before this patch.

  • slice now considers intro and outro: the recent change from insert to insertLeft/insertRight changed implementations to use intro/outro on a chunk, but slice wasn't updated to this new impl. Now, any intro/outro contained within the slice bounds is included in the result.
  • slice was broken if called after a removed or otherwise edited chunk: Because all chunks were being considered, regardless of if they overlapped the sliced range, a false-positive error was thrown indicating slicing a removed range. Now chunks which end before the sliced range are quickly skipped, removing the issue. As a bonus this adds a clarification of which bound was problematic.
  • slice was broken if chunks were moved such that chunks between the moved chunks would otherwise not be considered "included" in the slice range despite being between the start and end chunks.
  • performance of a slice of a small range suffers when in a large document: Previously all chunks were being considered, however the while loop can cease early as soon as a chunk is encountered which contains the end index.
  • slice API behavior differs slightly from Array#slice, that if a start is not provided, the empty string is returned, instead of the expected default 0 being used as start.

@leebyron leebyron force-pushed the fix-slice branch 7 times, most recently from f9ced19 to 1574db3 Compare May 13, 2016 01:29
This fixes a few issues with slice introduced recently. Tests are added which fail before this patch.

* slice now considers `intro` and `outro`: the recent change from `insert` to `insertLeft`/`insertRight` changed implementations to use `intro`/`outro` on a chunk, but slice wasn't updated to this new impl. Now, any `intro`/`outro` contained within the slice bounds is included in the result.

* slice was broken if called after a `removed` or otherwise `edited` chunk: Because all chunks were being considered, regardless of if they overlapped the sliced range, a false-positive error was thrown indicating slicing a removed range. Now chunks which end before the sliced range are quickly skipped, removing the issue. As a bonus this adds a clarification of *which* bound was problematic.

* slice was broken if chunks were moved such that chunks between the moved chunks would otherwise not be considered "included" in the slice range despite being between the start and end chunks.

* performance of a slice of a small range suffers when in a large document: Previously all chunks were being considered, however the while loop can cease early as soon as a chunk is encountered which starts after the sliced range.

* slice API behavior different slightly from Array#slice, that if a `start` is not provided, the empty string is provided, instead of the expected default `0` being used as `start`.
@Rich-Harris Rich-Harris merged commit 2266efb into Rich-Harris:master May 14, 2016
@Rich-Harris
Copy link
Owner

goddamn hero 👍

@Rich-Harris
Copy link
Owner

(released as 0.13.1)

@leebyron
Copy link
Contributor Author

🎉

@leebyron leebyron deleted the fix-slice branch May 14, 2016 19: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.

2 participants