ongoing windows username fix #2277
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mwestphal
force-pushed
the
fix_windows_username
branch
from
55d9ea7 to
f31234c
Compare
|
You are modifying libf3d public API! |
mwestphal
force-pushed
the
fix_windows_username
branch
from
f31234c to
dc6cc71
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -1122,7 +1122,7 @@ int F3DStarter::Start(int argc, char** argv) | |||
| } | |||
| } | |||
|
|
|||
| char* noDataForceRender = std::getenv("CTEST_F3D_NO_DATA_FORCE_RENDER"); | |||
| std::optional<std::string> noDataForceRender = f3d::utils::getEnv("CTEST_F3D_NO_DATA_FORCE_RENDER"); | |||
There was a problem hiding this comment.
Suggested change
| std::optional<std::string> noDataForceRender = f3d::utils::getEnv("CTEST_F3D_NO_DATA_FORCE_RENDER"); | |
| std::optional<std::string> noDataForceRender = | |
| f3d::utils::getEnv("CTEST_F3D_NO_DATA_FORCE_RENDER"); |
| const char* envPtr = std::getenv(envVar.c_str()); | ||
| if (!envPtr) | ||
| std::optional<std::string> envVal = f3d::utils::getEnv(envVar); | ||
| if (!envVal || envVal->empty() ) |
There was a problem hiding this comment.
Suggested change
| if (!envVal || envVal->empty() ) | |
| if (!envVal || envVal->empty()) |
| @@ -68,6 +69,26 @@ class F3D_EXPORT utils | |||
| [[nodiscard]] static std::filesystem::path collapsePath( | |||
| const std::filesystem::path& path, const std::filesystem::path& baseDirectory = {}); | |||
|
|
|||
| /** | |||
| * Get an environnement variable value, returns std::nullopt if not set | |||
There was a problem hiding this comment.
Suggested change
| * Get an environnement variable value, returns std::nullopt if not set | |
| * Get an environment variable value, returns std::nullopt if not set |
Comment on lines
+146
to
+148
| switch(knownFolder) | ||
| { | ||
| case KnownFolder::APPDATA: |
There was a problem hiding this comment.
Suggested change
| switch(knownFolder) | |
| { | |
| case KnownFolder::APPDATA: | |
| switch (knownFolder) | |
| case: | |
| KnownFolder::LOCALAPPDATA winKnowFolder = FOLDERID_LocalAppData; |
Meakk
reviewed
May 28, 2025
| { | ||
| return {}; | ||
| } | ||
| dirPath = fs::path(appData); | ||
| dirPath = fs::path(*appData); |
There was a problem hiding this comment.
It's better to be explicit imho here
Suggested change
| dirPath = fs::path(*appData); | |
| dirPath = fs::path(appData.value()); |
| */ | ||
| enum class KnownFolder : unsigned char | ||
| { | ||
| APPDATA, |
There was a problem hiding this comment.
Let's use the official name
Suggested change
| APPDATA, | |
| ROAMINGAPPDATA, |
| [[nodiscard]] static std::optional<std::string> getEnv(const std::string& env); | ||
|
|
||
| /** | ||
| * Enumeration of supported Windows known folders |
There was a problem hiding this comment.
Add the corresponding default values (%APPDATA% and %LOCALAPPDATA% respectively)
|
|
||
| #include <nlohmann/json.hpp> | ||
|
|
||
| /* |
Comment on lines
+135
to
+138
| else | ||
| { | ||
| return std::nullopt; | ||
| } |
There was a problem hiding this comment.
Suggested change
| else | |
| { | |
| return std::nullopt; | |
| } | |
| return std::nullopt; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Not ready for merge
Issue ticket number and link if any
#2084
Checklist for finalizing the PR
.github/workflows/versions.json, I have updatedtimestampContinuous integration
Please check the checkbox of the CI you want to run, then push again on your branch.