Fix Regex::replacen when replacing an empty match.
#395
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Note that i don't know why the CI checks are failing, something seemingly unrelated to this part of the code. |
|
Bump 🐅... Not sure if I should do anything or not right now! I guess someone would have to review this (actually this is really just a couple line, still I'm not sure about the simplicity of the code but now it's consistent). Note: I rebased on master to enable fast-forward. |
|
I just ran into this bug. This fix looks correct to me. @BurntSushi, r? |
|
Right, sorry, forgot about this. LGTM. Thanks so much for the bug fix! @bors r+ |
|
📌 Commit 408add0 has been approved by |
bors
added a commit
that referenced
this pull request
Nov 9, 2017
Fix `Regex::replacen` when replacing an empty match. Fixes #393, #394. There was some logic error when deciding to return `Cow::Borrowed` vs `Cow::Owned` in the fast-path. I took the same solution as the slow-path, ie use `peekable` on the iterator. I'm not sure this is the most efficient way (maybe just add `mut did_replace = false` and set it to true in the loop), but it does the job.
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #393, #394.
There was some logic error when deciding to return
Cow::BorrowedvsCow::Ownedin the fast-path. I took the same solution as the slow-path, ie usepeekableon the iterator. I'm not sure this is the most efficient way (maybe just addmut did_replace = falseand set it to true in the loop), but it does the job.