Skip to content

Split the search into values#825

Merged
Alkarex merged 1 commit intoFreshRSS:devfrom
aledeg:search-values
May 9, 2015
Merged

Split the search into values#825
Alkarex merged 1 commit intoFreshRSS:devfrom
aledeg:search-values

Conversation

@aledeg
Copy link
Copy Markdown
Member

@aledeg aledeg commented Apr 22, 2015

Before, the search was a single value.
Now it is splited in chuncks when separated by spaces.
Except if they are enclosed by single quotes or double quotes.

For some reasons, the unit tests are working for both single and double quotes but the
search box isn't. It is working only with single quotes.
We need to investigate the reason of this behavior.

See #823

Before, the search was a single value.
Now it is splited in chuncks when separated by spaces.
Except if they are enclosed by single quotes or double quotes.

For some reasons, the unit tests are working for both single and double quotes but the
search box isn't. It is working only with single quotes.
We need to investigate the reason of this behavior.

See FreshRSS#823
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have not tested yet, but just by reading quickly, isn't this regular expression greedy? I think it should search for the shortest "...", not the longest "..." ... "...". A suggestion is to replace the middle mask with (?P<search>*?)
Edit: Ah, sorry, I did not see the PCRE_UNGREEDY flag at the end.

Alkarex added a commit that referenced this pull request May 9, 2015
Split the search into values
@Alkarex Alkarex merged commit 71c4c3d into FreshRSS:dev May 9, 2015
@Alkarex
Copy link
Copy Markdown
Member

Alkarex commented May 9, 2015

Seems fine, thanks :-)

@Alkarex Alkarex added this to the 1.1.1 milestone May 16, 2015
@aledeg aledeg deleted the search-values branch June 6, 2015 13:45
@Alkarex Alkarex mentioned this pull request Jul 31, 2016
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.

2 participants