fix: resolve home directory via cross-platform helper (partial #68) - #87
Merged
Merged
Conversation
…v#68) InitCommand and YamlConfigProvider read \`getenv('HOME')\` directly to expand the leading \`~\` in the default config path. On Windows that env var is usually unset (the OS uses \`USERPROFILE\` plus optional \`HOMEDRIVE\` / \`HOMEPATH\` as the fallback chain), so \`~/.config/...\` quietly resolved to \`/.config/...\` at filesystem root and \`init\` / \`work:report\` silently misbehaved. Introduce \`Igancev\\WorkReporter\\Platform\\HomeDirectory::resolve()\` that returns the canonical home path per platform: \`HOME\` on POSIX, \`USERPROFILE\` on Windows with the documented \`HOMEDRIVE\` + \`HOMEPATH\` fallback for containers/services that strip \`USERPROFILE\`. The helper returns an empty string when nothing is set so callers see the same shape as the prior \`(string)getenv(...)\` cast. Scope: only the env-var leg of igancev#68's checklist. CI workflow for the Windows binary, README install section, and end-to-end Windows binary verification are intentionally left for follow-up PRs so each item stays atomic per CLAUDE.md / CONTRIBUTING.md guidance. Verification: vendor/bin/phpunit tests/Unit/Platform/HomeDirectoryTest.php vendor/bin/phpunit tests/Unit/InitCommandTest.php \ tests/Unit/Config/YamlConfigProviderTest.php make cs make stat-analyze all pass on macOS (POSIX path); the Windows-only assertions are skipped on POSIX runners and exercised on a future Windows CI lane.
Owner
|
Thanks for the PR, @mvanhorn, and sorry for the delay — I've been a bit busy lately. I'll go through your proposal soon. Really appreciate your contribution! |
igancev
approved these changes
Jun 2, 2026
Owner
|
Reviewed and merged - looks clean, nice refactoring. No critical issues found. Thanks for the contribution, @mvanhorn ! 👍 |
Contributor
Author
|
Appreciate the merge @igancev, the cross-platform home directory helper makes the reporter portable. |
Contributor
Author
|
Thanks @igancev! The cross-platform home directory helper should hold up on Windows too. |
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
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.
Summary
InitCommandandYamlConfigProviderreadgetenv('HOME')directly to expand the leading~in the default config path. On Windows that env var is usually unset (the OS usesUSERPROFILE, withHOMEDRIVE+HOMEPATHas a documented fallback for shells / containers that stripUSERPROFILE), so~/.config/work-reporter/...quietly resolved to/.config/work-reporter/...at the filesystem root andinit+work:reportsilently misbehaved.What changed
Igancev\WorkReporter\Platform\HomeDirectory::resolve()helper that returns the canonical home directory per platform:HOMEon POSIX,USERPROFILEon Windows with the documentedHOMEDRIVE+HOMEPATHfallback. Returns an empty string when nothing is set so callers see the same shape as the prior(string)getenv(...)cast (no behavior change in error reporting paths).src/Cli/InitCommand.php: swap the baregetenv('HOME')forHomeDirectory::resolve().src/Config/YamlConfigProvider.php: same swap.tests/Unit/Platform/HomeDirectoryTest.php: per-platform branches: POSIX returnsHOME, Windows prefersUSERPROFILEeven whenHOMEDRIVE+HOMEPATHare set, Windows falls back toHOMEDRIVE+HOMEPATHwhenUSERPROFILEis empty, and all-unset returns"". Each test skips on the other OS family; cross-OS assertions exercise on a future Windows CI lane.Scope
This is a partial fix for #68 — only the env-var handling item on the checklist. The remaining items (Windows binary build workflow, README install section, Windows binary smoke test) are intentionally left for follow-up PRs so each item stays atomic per CLAUDE.md / CONTRIBUTING.md guidance. Happy to take any of those in a follow-up if you want.
Verification
Local (macOS / PHP 8.5):
Closes part of #68 (env-var handling only).