Skip to content

Skip empty matches only when they overlap last match#145

Merged
iwillspeak merged 2 commits intorust-onig:mainfrom
sethp:fix/empty-matches
Sep 9, 2020
Merged

Skip empty matches only when they overlap last match#145
iwillspeak merged 2 commits intorust-onig:mainfrom
sethp:fix/empty-matches

Conversation

@sethp
Copy link
Copy Markdown
Contributor

@sethp sethp commented Jul 7, 2020

Previously, attempting to split around a delimiter would fail if that delimiter was preceded by any positive length match.

In my case, I was trying to use split to consume blank delimiters, but yield quote delimiters. I ended up writing a test like:

 assert_eq!(
        Regex::new(r#"(?=")|(?<=")| +"#)
            .unwrap()
            .split(" a\"b")
            .collect::<Vec<_>>(),
        vec!["", "a", "\"", "b"]
    );

But it was failing to split on the zero-width position before the quote:

thread 'option::test_onig' panicked at 'assertion failed: `(left == right)`
  left: `["", "a\"", "b"]`,
 right: `["", "a", "\"", "b"]`', src/option.rs:195:5

With this change, my test now passes. What do you think of it?

sethp added 2 commits July 7, 2020 08:50
Previously, attempting to `split` around a delimiter would fail if that delimiter was preceded by any positive length match.
@iwillspeak iwillspeak requested a review from a team September 8, 2020 07:31
@iwillspeak iwillspeak self-assigned this Sep 8, 2020
Copy link
Copy Markdown
Collaborator

@iwillspeak iwillspeak left a comment

Choose a reason for hiding this comment

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

This looks like a decent solution to the problem. I’m happy to merge.

@sethp
Copy link
Copy Markdown
Contributor Author

sethp commented Sep 8, 2020

Great, thanks for the review! Is there anything else you need from me on this PR?

@iwillspeak iwillspeak merged commit 99bcb06 into rust-onig:main Sep 9, 2020
@iwillspeak
Copy link
Copy Markdown
Collaborator

Was just waiting for the bump in minimum supported rust version before merging. This should
go out soon along with a few other tweaks as Onig 6.1.0.

iwillspeak added a commit that referenced this pull request Sep 9, 2020
Onig 6.1.0 and onig_sys 69.5.1 with:

 * Minimum Rust (MSRV) is now 1.40
 * WASM support (#147)
 * Empty matches are only skipped when they overlap the previous
   match (#145)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants