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

FI: Use std::filesystem #467

Closed
gpx1000 opened this issue May 31, 2022 · 2 comments · Fixed by #495
Closed

FI: Use std::filesystem #467

gpx1000 opened this issue May 31, 2022 · 2 comments · Fixed by #495
Labels
framework This is relevant to the framework

Comments

@gpx1000
Copy link
Contributor

gpx1000 commented May 31, 2022

#462 is a great implementation that depends upon platform specific code for filesystem classes (i.e. windowsFileSystem, unixFileSystem class etc). I would suggest that we can switch to using std::filesystem to provide a common class that takes away the need for platform dependent code.

@gpx1000 gpx1000 added the framework This is relevant to the framework label May 31, 2022
@TomAtkinsonArm
Copy link
Contributor

I like the idea of doing this. The main thing id want to keep is a standard way of defining virtual paths in the project. I guess now we are using c++17 we might be able to use https://en.cppreference.com/w/cpp/filesystem/path

I dont know enough information about this to implement it.

How hard would this be with the current #462? And can #462 merge without this change and the C++17 Filesystem be added as a separate PR?

@TomAtkinsonArm TomAtkinsonArm changed the title use std::filesystem for framework 2.0 FI: Use std::filesystem May 31, 2022
@gpx1000
Copy link
Contributor Author

gpx1000 commented May 31, 2022

I think it's fair to do it as a separate PR. It feels like it's a different task and it depends upon C++17 to get fully merged first so why add an extra hurdle to #462?

@TomAtkinsonArm TomAtkinsonArm linked a pull request Jul 10, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
framework This is relevant to the framework
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants