search

symphonists / search_index

Search Index provides an easy way to implement high performance fulltext searching on your Symphony site
32 stars 21 forks source link

preg_replace /e flag neccessary? #56

Closed animaux closed 7 years ago

animaux commented 7 years ago

Is the deprecated preg_replace /e flag neccessary in this line?

I have not got an error yet, but I’m not sure when exactly this is line run. I wonder if the /e flag is needed here, since there is no retrieved stored value in the replacement.

nitriques commented 7 years ago

Yes it is, it to call str_replace

str_replace(' ', 'SEARCH_INDEX_SPACE', '$0')

I do not know out to get the $0 value in the callback...

nitriques commented 7 years ago

Looks like it would be 

return str_replace(' ', 'SEARCH_INDEX_SPACE', $m[0]);

?

animaux commented 7 years ago

Hmm, thanks. I really would like to get an error before fixing this. Will try to find out when this is triggered.

animaux commented 7 years ago

The manipulateKeywords()-function that contains the questionable code is not referenced anywhere else in the extension, so would it be safe to say it’s not used? Probably some leftover? Or could it be invoked in any other way?

I’ve commented out the whole function declared here. I have yet to encounter an error. All seems to work well.

nitriques commented 7 years ago

@animaux You're right. Wanna send a PR with this deletion please ?

nitriques commented 7 years ago

I'll leave this open until it's fixed in master ;)

nitriques commented 7 years ago

Fixed in #55