Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Remove white space next to colon on keyword suggestion #1256

Merged
merged 1 commit into from

2 participants

@oiami
Collaborator

Fix issue #515 , done ! :)

@rwstauner
Owner

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

@oiami
Collaborator

I try your suggestion and found that

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

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

@rwstauner
Owner

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
Collaborator

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

@rwstauner rwstauner merged commit dfc9452 into CPAN-API:master
@rwstauner
Owner

Looks good. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 29, 2014
  1. @oiami
This page is out of date. Refresh to see the latest.
View
2  lib/MetaCPAN/Web/Controller/Search.pm
@@ -61,7 +61,7 @@ sub index : Path {
if ( !$results->{total} && !$authors->{total} ) {
my $suggest = $query;
- $suggest =~ s/:+/::/g;
+ $suggest =~ s/\s*:+\s*/::/g;
if ( $suggest ne $query ) {
$c->stash(
{
View
8 t/controller/search/suggestion.t
@@ -4,9 +4,11 @@ use Test::More;
use MetaCPAN::Web::Test;
my %tests = (
- 'DBIx:Class:::ResultSet' => 'DBIx::Class::ResultSet',
- 'DBIx::Class:ResultSet' => 'DBIx::Class::ResultSet',
- 'DBIx:Class' => 'DBIx::Class',
+ 'DBIx:Class:::ResultSet' => 'DBIx::Class::ResultSet',
+ 'DBIx::Class:ResultSet' => 'DBIx::Class::ResultSet',
+ 'DBIx:Class' => 'DBIx::Class',
+ 'DBIx: Class' => 'DBIx::Class',
+ 'DBIx ::Class: ResultSet' => 'DBIx::Class::ResultSet',
);
test_psgi app, sub {
Something went wrong with that request. Please try again.