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

Allow disabling std::filesystem support #3194

Closed
wants to merge 1 commit into from
Closed

Conversation

vadz
Copy link

@vadz vadz commented Dec 14, 2021

This can be useful when the compiler claims C++17 support, but standard
library std::filesystem implementation is unavailable (MinGW) or can't
be used because the current macOS target is too low to allow it (#3156).

See #3090.


I think it could be useful to disable this by default when macOS min version target is lower than 10.15 and will add a commit doing this if there are no objections.

This can be useful when the compiler claims C++17 support, but standard
library std::filesystem implementation is unavailable (MinGW) or can't
be used because the current macOS target is too low to allow it (nlohmann#3156).

See nlohmann#3090.
@nlohmann
Copy link
Owner

The filesystem issue is fixed here: #3101 - it also implements macros JSON_HAS_FILESYSTEM and JSON_HAS_EXPERIMENTAL_FILESYSTEM to switch off filesystem support altogether. That PR currently has a failing PR, but it solves the issue completely. Hence, I will probably not merge this PR.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 5c5fcd0 on vadz:no-std-fs into 7603046 on nlohmann:develop.

@vadz
Copy link
Author

vadz commented Dec 14, 2021

Sorry for wasting your time, I've somehow failed to find #3101 which is, of course, much more complete and better than this version, thanks for pointing me to it!

@vadz vadz closed this Dec 14, 2021
@vadz vadz deleted the no-std-fs branch December 14, 2021 19:06
@gregmarr
Copy link
Contributor

@vadz Thanks anyway for the contribution.

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.

None yet

4 participants