Remove white space next to colon on keyword suggestion #1256

merged 1 commit into from Jul 30, 2014


None yet

2 participants

oiami commented Jul 23, 2014

Fix issue #515 , done ! :)


Instead of (\s+)? how about \s*?
It's simpler and doesn't create captures that we aren't using.

oiami commented Jul 28, 2014

I try your suggestion and found that

  • s/\s*?:+(\s*)?/::/g
  • s/\s*?:+\s*?/::/g

I'm not good at regex, so not sure what make it behaves like that. What do you think ?


There are a few things going on here:

  • The star means "any number (0 or more) of this"
  • Following a quantifier (like the star) with a question mark makes it non-greedy (which means it matches the fewest possible instead of the most possible)
  • The parentheses make a capture group, which saves the matched text into a variable and allows other operators to work on the group.
  • The question mark after the capture group means the group doesn't actually have to match (technically it can match zero or one time).

So the (\s+)? means that the group must capture one or more whitespace characters, but the question mark then says that the capture group doesn't have to match in order for the regexp as a whole to match.

Since we don't actually care about the whitespace we don't need to capture it. It leads to some confusion in reading the code.

Instead, s/\s*:+\s*/::/g should do what you want:
Match any number of colons, which might (or might not) be surrounded by any number of whitespace characters (zero or more), discard the whitespace, and make the number of colons exactly two.

oiami commented Jul 29, 2014

That's really useful, I will print it out and stick on the wall 👍 , pushed new fixed regex and squashed it. Thanks a lot for your suggestion :D

@rwstauner rwstauner merged commit dfc9452 into metacpan:master Jul 30, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build could not complete due to an error

Looks good. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment