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

FileSearch: Don't use RegExes, just do string comparisons. #3075

Merged
merged 2 commits into from Oct 1, 2015

Conversation

waddlesplash
Copy link
Contributor

Nothing used the RegEx feature of FileSearch, and GCC < 4.9 doesn't support C++11 RegEx properly, so get rid of it.

CC @comex as he wrote the feature & @lioncash for code review

}
regex_str += ")$";
std::regex regex(regex_str, std::regex_constants::icase);
bool accept_all = (exts.size() == 0) || (std::find(exts.begin(), exts.end(), "") != exts.end());

This comment was marked as off-topic.

@@ -162,7 +162,7 @@ void InterfaceConfigPane::LoadGUIValues()

void InterfaceConfigPane::LoadThemes()
{
auto sv = DoFileSearch({"*"}, {

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@comex
Copy link
Contributor

comex commented Sep 27, 2015

why, why, why do we care about GCC < 4.9 :(

but I guess it isn't the end of the world if it wasn't being used anyway.

@waddlesplash
Copy link
Contributor Author

why, why, why do we care about GCC < 4.9 :(

This should be a performance boost (though it may not be too noticeable) as well as supporting GCC < 4.9. And I want this to get Dolphin working on Haiku, as Haiku finally has EGL support, but is still on GCC 4.8 for the time being.

@waddlesplash
Copy link
Contributor Author

Sorry, clarification - it'll only be a performance boost if the regex evaluator doesn't have a JIT. I'm not aware of any implementations that do have a JIT right now.

@JosJuice
Copy link
Member

If you want to support GCC 4.8, aren't there more things you need to change? For instance, PR #2674 (and emplaces added after that PR was made)

@comex
Copy link
Contributor

comex commented Sep 27, 2015

It's not a performance boost because the cost of doing a filename comparison is negligible - only the time it takes the OS to hand over the directory listing matters.

I guess 17 months isn't that long for Haiku to drag feet...

@waddlesplash
Copy link
Contributor Author

If you want to support GCC 4.8, aren't there more things you need to change? For instance, PR #2674 (and emplaces added after that PR was made)

Hmm, not sure. I know that SymbolDB at least compiled on the offline WIP I have with no errors or warnings, so I don't know...

@waddlesplash
Copy link
Contributor Author

@lioncash
Copy link
Member

Even if it didn't, it wouldn't be reverted just to support Haiku.

@waddlesplash
Copy link
Contributor Author

I realize that.

@waddlesplash
Copy link
Contributor Author

@comex fixed.

return true;
std::string name = entry.virtualName;
std::transform(name.begin(), name.end(), name.begin(), ::tolower);
return std::any_of(exts.begin(), exts.end(), [&](std::string ext) {

This comment was marked as off-topic.

Nothing used the RegEx feature of FileSearch, and GCC < 4.9
doesn't support C++11 RegEx properly, so get rid of it.
@waddlesplash
Copy link
Contributor Author

@lioncash fixed.

@phire
Copy link
Member

phire commented Oct 1, 2015

No guarantee that gcc 4.8 will stay supported in the future, but LGTM.

phire added a commit that referenced this pull request Oct 1, 2015
FileSearch: Don't use RegExes, just do string comparisons.
@phire phire merged commit 58d893e into dolphin-emu:master Oct 1, 2015
@waddlesplash waddlesplash deleted the no-regexp branch October 1, 2015 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants