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

Deal with regex match groups in excerpt #15899

Merged
merged 1 commit into from
Jun 24, 2014

Conversation

garethrees
Copy link
Contributor

Original implementation has bugs if the regex contains a match group.

Example:

excerpt('This is a beautiful? morning', /\b(beau\w*)\b/i, :radius => 5)
Expected: "...is a beautiful? mor..."
Actual: "...is a beautifulbeaut..."

The original phrase was being converted to a regex and returning the text
either side of the phrase as expected:

'This is a beautiful? morning'.split(/beautiful/i, 2)
# => ["This is a ", "? morning"]

When we have a match with groups the match is returned in the array.

Quoting the ruby docs: "If pattern is a Regexp, str is divided where the
pattern matches. [...] If pattern contains groups, the respective matches will
be returned in the array as well."

'This is a beautiful? morning'.split(/\b(beau\w*)\b/iu, 2)
# => ["This is a ", "beautiful", "? morning"]

If we assume we want to split on the first match – this fix makes that
assumption – we can pass the already assigned phrase variable as the place
to split (because we already know that a match exists from line 168).

Originally spotted by Louise Crow (@crowbot) at
mysociety/alaveteli#1557

Original implementation has bugs if the regex contains a match group.

Example:

    excerpt('This is a beautiful? morning', /\b(beau\w*)\b/i, :radius => 5)
    Expected: "...is a beautiful? mor..."
    Actual: "...is a beautifulbeaut..."

The original phrase was being converted to a regex and returning the text
either side of the phrase as expected:

    'This is a beautiful? morning'.split(/beautiful/i, 2)
    # => ["This is a ", "? morning"]

When we have a match with groups the match is returned in the array.

Quoting the ruby docs: "If pattern is a Regexp, str is divided where the
pattern matches. [...] If pattern contains groups, the respective matches will
be returned in the array as well."

    'This is a beautiful? morning'.split(/\b(beau\w*)\b/iu, 2)
    # => ["This is a ", "beautiful", "? morning"]

If we assume we want to split on the first match – this fix makes that
assumption – we can pass the already assigned `phrase` variable as the place
to split (because we already know that a match exists from line 168).

Originally spotted by Louise Crow (@crowbot) at
mysociety/alaveteli#1557
rafaelfranca added a commit that referenced this pull request Jun 24, 2014
Deal with regex match groups in excerpt
@rafaelfranca rafaelfranca merged commit 44c9489 into rails:master Jun 24, 2014
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

2 participants