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

[bytes] fix bytesFindIndex and bytesFindLastIndex #381

Merged
merged 5 commits into from
May 7, 2019

Conversation

arcatdmz
Copy link
Contributor

@arcatdmz arcatdmz commented May 5, 2019

Bug

bytesFindIndex and bytesFindLastIndex in deno_std/bytes cannot find the binary pattern when its occurrence has an overlap with a partly (but not completely) matching sequence.

  • target: 001
  • pattern: 01
  • expected bytesFindIndex result: 1
  • current bytesFindIndex result: -1

Reproduction

Existing test cases are edited to reproduce the bug: arcatdmz@fe9d5b4

Two corresponding tests fail accordingly: https://gist.github.com/arcatdmz/08ae107dd5e29e2aaebf18e1d8b76d43

Fix

This pull request contains a commit that fixes the bug: arcatdmz@1251423

Context

@CLAassistant
Copy link

CLAassistant commented May 5, 2019

CLA assistant check
All committers have signed the CLA.

deno --allow-read --allow-write prettier/main.ts --ignore node_modules --ignore testdata --ignore vendor
Copy link
Member

@ry ry 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 fix @arcatdmz ! I just one comment...

@keroxp please review if you have time.

bytes/bytes_test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@keroxp keroxp left a comment

Choose a reason for hiding this comment

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

Nice work. LGTM

Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

Thanks!

@ry ry merged commit 8714252 into denoland:master May 7, 2019
arcatdmz added a commit to arcatdmz/danoweb that referenced this pull request May 7, 2019
@arcatdmz arcatdmz deleted the fix-bytes-bytesFindIndex branch May 15, 2019 22:02
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.

None yet

4 participants