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 Bug in rure_captures_len #365

Merged
merged 1 commit into from
May 14, 2017
Merged

Fix Bug in rure_captures_len #365

merged 1 commit into from
May 14, 2017

Conversation

iwillspeak
Copy link
Contributor

It looks like at some point in the past the captures were refactored
from being a vector of start and end positions into a list of location
structures. The C API still had a conversion of the lenght which
corrected for the captures being twice the length of the number of
captures.

This updates the length calculation in rure.rs to return the
correct length, and adds an assertion to the test case.

@BurntSushi
Copy link
Member

How embarrassing! Thank you for the fox and the test.

I'm on mobile so it's hard to tell, but it looks like the C code is formatted differently from other C code in the same file. Could you make the formatting consistent? (And please also squash. :-)) Thanks!

@iwillspeak
Copy link
Contributor Author

iwillspeak commented May 14, 2017 via email

It looks like at some point in the past the captures were refactored
from being a vector of start and end positions into a list of location
structures. The C API still had a conversion of the length which
corrected for the captures being twice the length of the number of
captures.

This updates the length calculation in `rure.rs` to return the
correct length, and adds an assertion to the test case.
@BurntSushi
Copy link
Member

Awesome, thank you! @bors r+

@bors
Copy link
Contributor

bors commented May 14, 2017

📌 Commit 4ed2fed has been approved by BurntSushi

@bors
Copy link
Contributor

bors commented May 14, 2017

⌛ Testing commit 4ed2fed with merge 68bc958...

bors added a commit that referenced this pull request May 14, 2017
Fix Bug in `rure_captures_len`

It looks like at some point in the past the captures were refactored
from being a vector of start and end positions into a list of location
structures. The C API still had a conversion of the lenght which
corrected for the captures being twice the length of the number of
captures.

This updates the length calculation in `rure.rs` to return the
correct length, and adds an assertion to the test case.
@bors
Copy link
Contributor

bors commented May 14, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: BurntSushi
Pushing 68bc958 to master...

@bors bors merged commit 4ed2fed into rust-lang:master May 14, 2017
@iwillspeak
Copy link
Contributor Author

\o/

@iwillspeak iwillspeak deleted the capture-len-fix branch May 14, 2017 14:45
iwillspeak added a commit to iwillspeak/IronRure-Batteries that referenced this pull request May 15, 2017
iwillspeak added a commit to iwillspeak/IronRure that referenced this pull request May 15, 2017
Gets the lastst versions of the native packages. These builds include
the fix for rust-lang/regex#365.
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.

3 participants