move default config file from userprofile\.config to scoop\persist - Update core.ps1#6600
move default config file from userprofile\.config to scoop\persist - Update core.ps1#660029039 wants to merge 1 commit intoScoopInstaller:masterfrom
Conversation
WalkthroughIntroduces legacy config handling to Scoop's config loading system by adding XDG_CONFIG_HOME, legacyConfig, and persist-based path search order. Adjusts config file resolution to use the new configHome-derived path while preserving portable-config behavior through portable path detection. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/core.ps1`:
- Line 1263: The current logic sets $legacyConfig by testing the existence of
the directory "$env:USERPROFILE\.config" which prefers the legacy directory even
when the actual config file "$env:USERPROFILE\.config\scoop\config.json" is
missing; update the check so $legacyConfig points to the actual config file path
(e.g., compose the full path to "scoop\config.json" under
$env:USERPROFILE\.config) and use Test-Path against that file; similarly ensure
$configHome selection logic treats the persist path and legacy file consistently
(check for config file existence rather than just directory) so loading returns
the real config path or falls back to persist when the file is absent.
- Around line 1263-1265: $configHome can become $null because Select-Object
-First 1 returns the first element even if it's $null; update the assignment for
$configHome (which is used to build $configFile) to filter out empty/null/falsy
values before selecting the first candidate (follow the same pattern used for
$scoopdir): build the candidate list including $env:XDG_CONFIG_HOME,
$legacyConfig, and the fallback, pipe it through a Where-Object filter that
removes empty/null entries, then Select-Object -First 1 so $configHome is never
set to $null when XDG_CONFIG_HOME is unset.
🧹 Nitpick comments (1)
lib/core.ps1 (1)
1264-1264: The persist fallback path is not resolved to an absolute path.
"$PSScriptRoot\..\..\..\..\persist"remains a relative-style path with..segments. Other path variables in this file (e.g.,$scoopdiron Line 1292) are piped throughGet-AbsolutePathto normalize them. Since$configFileis derived from$configHomeand later used in file I/O (load_cfg,Out-UTF8File), leaving it un-normalized could cause subtle issues with path comparisons or display.Proposed fix
-$configHome = $env:XDG_CONFIG_HOME, $legacyConfig, "$PSScriptRoot\..\..\..\..\persist" | Where-Object { $_ } | Select-Object -First 1 +$configHome = $env:XDG_CONFIG_HOME, $legacyConfig, "$PSScriptRoot\..\..\..\..\persist" | Where-Object { $_ } | Select-Object -First 1 | Get-AbsolutePath
|
|
||
| # Load Scoop config | ||
| $configHome = $env:XDG_CONFIG_HOME, "$env:USERPROFILE\.config" | Select-Object -First 1 | ||
| $legacyConfig = if (Test-Path "$env:USERPROFILE\.config") { "$env:USERPROFILE\.config" } |
There was a problem hiding this comment.
$legacyConfig checks directory existence but the path semantics have shifted.
The legacy path "$env:USERPROFILE\.config" is a directory, while the new persist path on Line 1264 ("$PSScriptRoot\..\..\..\..\persist") is also a directory — both used as a config home prefix for "$configHome\scoop\config.json". However, the legacy .config directory is validated with Test-Path while the persist path is not. A non-existent persist directory would still be selected as the fallback, which may be intentional (config will be created there), but it means the legacy config is only preferred if the directory exists, not if the config file inside it exists.
If a user has an empty ~\.config directory (common on Windows with cross-platform tools) but no ~\.config\scoop\config.json, this will incorrectly select the legacy path over persist, and config loading will silently return $null.
Consider checking for the actual config file instead:
Proposed fix
-$legacyConfig = if (Test-Path "$env:USERPROFILE\.config") { "$env:USERPROFILE\.config" }
+$legacyConfig = if (Test-Path "$env:USERPROFILE\.config\scoop\config.json") { "$env:USERPROFILE\.config" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $legacyConfig = if (Test-Path "$env:USERPROFILE\.config") { "$env:USERPROFILE\.config" } | |
| $legacyConfig = if (Test-Path "$env:USERPROFILE\.config\scoop\config.json") { "$env:USERPROFILE\.config" } |
🧰 Tools
🪛 PSScriptAnalyzer (1.23.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'core.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
In `@lib/core.ps1` at line 1263, The current logic sets $legacyConfig by testing
the existence of the directory "$env:USERPROFILE\.config" which prefers the
legacy directory even when the actual config file
"$env:USERPROFILE\.config\scoop\config.json" is missing; update the check so
$legacyConfig points to the actual config file path (e.g., compose the full path
to "scoop\config.json" under $env:USERPROFILE\.config) and use Test-Path against
that file; similarly ensure $configHome selection logic treats the persist path
and legacy file consistently (check for config file existence rather than just
directory) so loading returns the real config path or falls back to persist when
the file is absent.
| $legacyConfig = if (Test-Path "$env:USERPROFILE\.config") { "$env:USERPROFILE\.config" } | ||
| $configHome = $env:XDG_CONFIG_HOME, $legacyConfig, "$PSScriptRoot\..\..\..\..\persist" | Select-Object -First 1 | ||
| $configFile = "$configHome\scoop\config.json" |
There was a problem hiding this comment.
Bug: $configHome will be $null when $env:XDG_CONFIG_HOME is unset.
Select-Object -First 1 picks the first array element regardless of whether it's $null. Since most users won't have XDG_CONFIG_HOME set, $configHome resolves to $null and $configFile becomes "\scoop\config.json" (a relative path from the filesystem root), breaking config loading entirely.
Follow the existing pattern used on Line 1292 for $scoopdir — filter out falsy values first:
Proposed fix
-$configHome = $env:XDG_CONFIG_HOME, $legacyConfig, "$PSScriptRoot\..\..\..\..\persist" | Select-Object -First 1
+$configHome = $env:XDG_CONFIG_HOME, $legacyConfig, "$PSScriptRoot\..\..\..\..\persist" | Where-Object { $_ } | Select-Object -First 1📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $legacyConfig = if (Test-Path "$env:USERPROFILE\.config") { "$env:USERPROFILE\.config" } | |
| $configHome = $env:XDG_CONFIG_HOME, $legacyConfig, "$PSScriptRoot\..\..\..\..\persist" | Select-Object -First 1 | |
| $configFile = "$configHome\scoop\config.json" | |
| $legacyConfig = if (Test-Path "$env:USERPROFILE\.config") { "$env:USERPROFILE\.config" } | |
| $configHome = $env:XDG_CONFIG_HOME, $legacyConfig, "$PSScriptRoot\..\..\..\..\persist" | Where-Object { $_ } | Select-Object -First 1 | |
| $configFile = "$configHome\scoop\config.json" |
🧰 Tools
🪛 PSScriptAnalyzer (1.23.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'core.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
In `@lib/core.ps1` around lines 1263 - 1265, $configHome can become $null because
Select-Object -First 1 returns the first element even if it's $null; update the
assignment for $configHome (which is used to build $configFile) to filter out
empty/null/falsy values before selecting the first candidate (follow the same
pattern used for $scoopdir): build the candidate list including
$env:XDG_CONFIG_HOME, $legacyConfig, and the fallback, pipe it through a
Where-Object filter that removes empty/null entries, then Select-Object -First 1
so $configHome is never set to $null when XDG_CONFIG_HOME is unset.
|
There are documentation considerations in the following files but maybe it would be excessive to be doing pull requests for each one: libexec/scoop-checkup.ps1 libexec/scoop-config.ps1 .github/ISSUE_TEMPLATE/Bug_report.md Also I think that the coderabbit analysis is wrong - | Select-Object -First 1 skips over the null objects, that is how it was before, and also tested this to be the case. |
|
|
Oh great! Can we please make this the default as to not litter the home directory with extra folders which don't serve much purpose? To be honest, I already find it quite dubious that Scoop by default I installed to ~\scoop rather than %LocalAppData%\scoop like any other user app would do. |
2 participants
Description
dumping a .config folder in the root user profile is annoying, so I propose moving it to be in scoop\persist\scoop to be consistent with other scoop apps
Motivation and Context
Annoyance
Companion to ScoopInstaller/Install#126
How Has This Been Tested?
tested on my PC works fine
tested to still accept .config as priority if it already exists
Checklist:
developbranch.Sorry am noob
Summary by CodeRabbit