-
Notifications
You must be signed in to change notification settings - Fork 399
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
Migrate various APIs to fs::path #7151
base: main
Are you sure you want to change the base?
Conversation
It's different on Windows; a native string is wide. So on Windows this would just be an infinite recursion, rather than what we actually want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! One copy we don’t need
@@ -245,25 +247,20 @@ bool FxUserPreset::hasPresetsForSingleType(int id) | |||
return scannedPresets.find(id) != scannedPresets.end(); | |||
} | |||
|
|||
void FxUserPreset::saveFxIn(SurgeStorage *storage, FxStorage *fx, const std::string &s) | |||
void FxUserPreset::saveFxIn(SurgeStorage *storage, FxStorage *fx, fs::path sp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be const fs::path &
@@ -72,7 +72,7 @@ struct FxUserPreset | |||
bool hasPresetsForSingleType(int type_id); | |||
bool readFromXMLSnapshot(Preset &p, TiXmlElement *); | |||
|
|||
void saveFxIn(SurgeStorage *s, FxStorage *fxdata, const std::string &fn); | |||
void saveFxIn(SurgeStorage *s, FxStorage *fxdata, fs::path sp); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Const & again to avoid thencopy
DO NOT MERGE this yet. This pull request will end up with several more commits as part of it. I'm opening it early to test Windows CI.