-
-
Notifications
You must be signed in to change notification settings - Fork 132
feat: use portable config #119
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughReplaces XDG/USERPROFILE-based config path with SCOOP_DIR-derived Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant I as install.ps1
participant G as Git Host
participant Z as Zip Host
U->>I: Run installer
rect rgba(240,240,255,0.6)
I->>I: Resolve config paths\nif SCOOP_DIR -> SCOOP_CONFIG_HOME=SCOOP_DIR\nSCOOP_CONFIG_FILE=SCOOP_DIR\config.json\nelse -> use XDG/USERPROFILE
end
rect rgba(235,255,235,0.6)
I->>G: Attempt clone `SCOOP_PACKAGE_GIT_REPO` (if set & git available)
alt clone succeeds
I->>G: Clone `SCOOP_MAIN_BUCKET_GIT_REPO` (if set)
else clone fails
I->>Z: Download package ZIP (`SCOOP_PACKAGE_REPO`)
I->>Z: Download main bucket ZIP (`SCOOP_MAIN_BUCKET_REPO`)
end
end
I-->>U: Bootstrap complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
install.ps1 (1)
595-603: Git clone path is missing parent directoriesLine 595 calls
git clonebefore ensuring that$SCOOP_APP_DIR/$SCOOP_MAIN_BUCKET_DIRparents exist. On a clean machineapps\scoopandbucketsaren’t there yet, so Git fails with “could not create leading directories…”, and we always fall back to the zip path. Please create the parent folders first so the new Git bootstrap actually succeeds.+ $appParentDir = Split-Path -Parent $SCOOP_APP_DIR + if (!(Test-Path $appParentDir)) { + New-Item -ItemType Directory -Path $appParentDir -Force | Out-Null + } + $mainBucketParentDir = Split-Path -Parent $SCOOP_MAIN_BUCKET_DIR + if (!(Test-Path $mainBucketParentDir)) { + New-Item -ItemType Directory -Path $mainBucketParentDir -Force | Out-Null + } Write-Verbose "Cloning $SCOOP_PACKAGE_GIT_REPO to $SCOOP_APP_DIR" git clone -q $SCOOP_PACKAGE_GIT_REPO $SCOOP_APP_DIR
install.ps1
Outdated
| if ($SCOOP_DIR) { | ||
| $SCOOP_CONFIG_HOME = $SCOOP_DIR | ||
| $SCOOP_CONFIG_FILE = "$SCOOP_DIR\config.json" | ||
| } else { | ||
| $SCOOP_CONFIG_HOME = $env:XDG_CONFIG_HOME, "$env:USERPROFILE\.config" | Select-Object -First 1 | ||
| $SCOOP_CONFIG_FILE = "$SCOOP_CONFIG_HOME\scoop\config.json" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Portable-config condition always triggers
Because Line 685 already guarantees $SCOOP_DIR is a non-empty string (it falls back to $env:USERPROFILE\scoop), the new if ($SCOOP_DIR) branch always runs. That means every install now writes config.json into $SCOOP_DIR, breaking the previous default of $XDG_CONFIG_HOME\scoop\config.json/%USERPROFILE%\.config\scoop\config.json. We only want the portable layout when the caller explicitly set ScoopDir or $env:SCOOP.
-if ($SCOOP_DIR) {
+$usePortableConfig = $PSBoundParameters.ContainsKey('ScoopDir') -or -not [string]::IsNullOrEmpty($env:SCOOP)
+if ($usePortableConfig) {
$SCOOP_CONFIG_HOME = $SCOOP_DIR
$SCOOP_CONFIG_FILE = "$SCOOP_DIR\config.json"
} else {
$SCOOP_CONFIG_HOME = $env:XDG_CONFIG_HOME, "$env:USERPROFILE\.config" | Select-Object -First 1
$SCOOP_CONFIG_FILE = "$SCOOP_CONFIG_HOME\scoop\config.json"
}📝 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.
| if ($SCOOP_DIR) { | |
| $SCOOP_CONFIG_HOME = $SCOOP_DIR | |
| $SCOOP_CONFIG_FILE = "$SCOOP_DIR\config.json" | |
| } else { | |
| $SCOOP_CONFIG_HOME = $env:XDG_CONFIG_HOME, "$env:USERPROFILE\.config" | Select-Object -First 1 | |
| $SCOOP_CONFIG_FILE = "$SCOOP_CONFIG_HOME\scoop\config.json" | |
| } | |
| # only use the "portable" config layout when ScoopDir was passed or $env:SCOOP is set | |
| $usePortableConfig = $PSBoundParameters.ContainsKey('ScoopDir') -or -not [string]::IsNullOrEmpty($env:SCOOP) | |
| if ($usePortableConfig) { | |
| $SCOOP_CONFIG_HOME = $SCOOP_DIR | |
| $SCOOP_CONFIG_FILE = "$SCOOP_DIR\config.json" | |
| } else { | |
| $SCOOP_CONFIG_HOME = $env:XDG_CONFIG_HOME, "$env:USERPROFILE\.config" | Select-Object -First 1 | |
| $SCOOP_CONFIG_FILE = "$SCOOP_CONFIG_HOME\scoop\config.json" | |
| } |
🤖 Prompt for AI Agents
In install.ps1 around lines 697 to 703, the current if ($SCOOP_DIR) always
evaluates true because $SCOOP_DIR was earlier defaulted, so change the condition
to only use the portable layout when the caller explicitly set ScoopDir or
$env:SCOOP; replace the if with a check like: if ($env:SCOOP -or
($PSBoundParameters.ContainsKey('ScoopDir'))) { ... } else { ... } so that when
neither the env var nor the script parameter was provided the script falls back
to XDG/USERPROFILE config paths.
0faf2d8 to
da42fb0
Compare
|
Since Scoop supports portable configurations, it should be used by default, as it is more in line with Scoop. |
Summary by CodeRabbit
New Features
Changed