-
Notifications
You must be signed in to change notification settings - Fork 1.5k
smarttaskbar@1.4.5: Improve startup script handling, update notes #16492
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
WalkthroughThe SmartTaskbar package manifest was reorganized to reflect repository ownership change and fixes. The installer was refactored from a fixed script to a templated, loop-driven approach that substitutes placeholders for startup directory paths. Two new PowerShell scripts were introduced for adding and removing startup shortcuts. Metadata updates include expanded notes, updated homepage and license format, and removal of the persist field. Changes
Sequence DiagramsequenceDiagram
participant Installer as Scoop Installer
participant Template as Templated Installer Script
participant Script as Startup Scripts<br/>(add/remove-smarttaskbar-startup)
participant FS as File System
Installer->>Template: Process manifest installer<br/>with directory placeholders
Template->>Script: Loop through startup scripts
Template->>Script: Substitute {{startup_dir}}<br/>& {{current_dir}} placeholders
Template->>FS: Write resolved scripts
alt User runs add-smarttaskbar-startup
Script->>FS: Create WScript.Shell COM object
Script->>FS: Create shortcut at<br/>{{startup_dir}}\SmartTaskbar.lnk
Script->>FS: Set TargetPath to<br/>{{current_dir}}\SmartTaskbar.exe
else User runs remove-smarttaskbar-startup
Script->>FS: Delete {{startup_dir}}\SmartTaskbar.lnk<br/>with -Force flag
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Areas for attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
|
All changes look good. Wait for review from human collaborators. smarttaskbar
|
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: 2
🧹 Nitpick comments (2)
scripts/smarttaskbar/remove-startup.ps1 (1)
1-2: LGTM! Consider optional error handling improvement.The script correctly removes the startup shortcut with
-ErrorAction SilentlyContinueto avoid errors when the file doesn't exist. The success message always displays, which is acceptable for typical scenarios.If you want more precise feedback, consider capturing the result and conditionally printing the message:
-Remove-Item -Path '{{startup_dir}}\SmartTaskbar.lnk' -Force -ErrorAction SilentlyContinue -Write-Host "SmartTaskbar has been removed from the startup programs." -ForegroundColor Green +$removed = Remove-Item -Path '{{startup_dir}}\SmartTaskbar.lnk' -Force -ErrorAction SilentlyContinue -PassThru +if ($removed -or !(Test-Path '{{startup_dir}}\SmartTaskbar.lnk')) { + Write-Host "SmartTaskbar has been removed from the startup programs." -ForegroundColor Green +}bucket/smarttaskbar.json (1)
26-29: Consider using UTF8 encoding instead of Ascii.The script uses
-Encoding Asciifor reading and writing files, which could cause issues with non-ASCII characters (though unlikely in these specific scripts). Modern PowerShell best practices recommend UTF8 encoding.Apply this diff to use UTF8 encoding:
- $content = Get-Content -Path $startup_script -Encoding Ascii + $content = Get-Content -Path $startup_script -Encoding UTF8 $content = $content -replace '{{startup_dir}}', $startup_dir $content = $content -replace '{{current_dir}}', $current_dir - $content | Set-Content -Path "$dir\\$_.ps1" -Encoding Ascii + $content | Set-Content -Path "$dir\\$_.ps1" -Encoding UTF8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bucket/smarttaskbar.json(1 hunks)scripts/smarttaskbar/add-startup.ps1(1 hunks)scripts/smarttaskbar/remove-startup.ps1(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (4)
bucket/smarttaskbar.json (4)
3-8: LGTM! Metadata correctly updated.The description, homepage, and license fields have been properly updated to reflect the new repository (Oliviaophia/SmartTaskbar). The license object structure follows manifest best practices.
9-15: LGTM! Helpful user guidance provided.The notes section is well-structured with clear instructions for managing startup programs and troubleshooting guidance for Windows Explorer-related issues during updates or uninstalls.
34-37: LGTM! Bin array correctly configured.The bin array properly includes both startup management scripts alongside the main executable. Users will be able to run
add-startupandremove-startupcommands as documented in the notes section.
45-47: LGTM! Uninstaller ensures proper cleanup.The uninstaller correctly references
remove-startup.ps1to clean up the startup shortcut when SmartTaskbar is uninstalled, ensuring no orphaned entries remain.
| "notes": [ | ||
| "Run 'add-startup' to add SmartTaskbar to startup programs.", | ||
| "Run 'remove-startup' to remove SmartTaskbar from startup programs.", | ||
| "", | ||
| "If an error occurs when updating or uninstalling, execute the following command then retry:", | ||
| "- `Stop-Process -Name 'explorer' -Force -ErrorAction SilentlyContinue`" | ||
| ], |
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.
Would adding it to startup via the registry work? If so, you can refer to Tailscale as an example.
This allows handling global installation while avoiding using the bin field.
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.
In theory, this approach is feasible. However, since I am not a user of this software, I prefer not to spend too much time testing an application I do not personally use.
Additionally, the installer configures auto-startup by adding a shortcut to Shell:Startup, and I believe there is no need to modify this behavior.
6649dee to
baac2d6
Compare
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
♻️ Duplicate comments (1)
scripts/smarttaskbar/add-smarttaskbar-startup.ps1 (1)
1-6: Critical issue already flagged: Missing error handling for Save() operation.The past review comment correctly identified that this script reports success unconditionally without verifying
Save()succeeded, reproducing the original issue #13256 where "add-startup script returns errors but reports success" andUnauthorizedAccessExceptionwas observed. The suggested try-catch wrapper with proper error propagation is the correct fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bucket/smarttaskbar.json(1 hunks)scripts/smarttaskbar/add-smarttaskbar-startup.ps1(1 hunks)scripts/smarttaskbar/remove-smarttaskbar-startup.ps1(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-29T15:04:21.650Z
Learnt from: SorYoshino
PR: ScoopInstaller/Extras#16492
File: bucket/smarttaskbar.json:21-21
Timestamp: 2025-10-29T15:04:21.650Z
Learning: In Scoop manifests, the shortcut_folder function returns a plain string (the path to the shortcut folder), not an object. Do not use Select-Object -ExpandProperty Path on its output, as this will cause a "Property 'Path' cannot be found" error. The correct usage is to pipe shortcut_folder directly to other cmdlets like Split-Path: `shortcut_folder $global | Split-Path -Parent`.
Applied to files:
bucket/smarttaskbar.json
📚 Learning: 2025-10-13T09:37:06.093Z
Learnt from: o-l-a-v
PR: ScoopInstaller/Extras#16328
File: bucket/winutil.json:0-0
Timestamp: 2025-10-13T09:37:06.093Z
Learning: When reviewing code in Scoop manifests, always verify the actual file content if the diff appears incomplete or unusual, as diff context can be truncated or displayed incorrectly. The pattern `[string[]](...).ForEach{...}` is commonly used in post_uninstall scripts across the Extras repository.
Applied to files:
bucket/smarttaskbar.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (6)
bucket/smarttaskbar.json (6)
3-8: LGTM: Metadata updates align with repository migration.The description, homepage, and license fields are correctly updated to reflect the new Oliviaophia/SmartTaskbar repository.
9-15: LGTM: Clear usage instructions and troubleshooting guidance.The notes provide accurate command names (matching the shimmed bin entries) and include helpful troubleshooting for the Explorer-related errors mentioned in the PR objectives.
21-21: LGTM: Correct fix for the "Property 'Path' cannot be found" bug.Removing
Select-Object -ExpandProperty Pathresolves the installer error from issue #13256. Theshortcut_folderfunction returns a plain string, so piping directly toSplit-Pathis the correct approach.Based on learnings.
34-38: LGTM: bin entries correctly reference the processed scripts.The bin entries include the .ps1 files that will be generated in
$dirby the installer script. These will be shimmed and accessible asadd-smarttaskbar-startupandremove-smarttaskbar-startupcommands.
23-31: No issues found — script files exist and naming is consistent.Verification confirms both template scripts exist at the expected locations and match the loop iteration values. The post_uninstall script path construction is correct.
45-47: No issues found—uninstaller file reference is correctly configured.The manifest properly handles the uninstaller file path:
- The installer script copies
remove-smarttaskbar-startup.ps1fromscripts/smarttaskbar/to the app installation directory ($dir)- The script is listed in the "bin" array, confirming installation
- The uninstaller correctly references
"remove-smarttaskbar-startup.ps1"by filename; Scoop automatically resolves this to the installation directory during uninstallationThis pattern is consistent with other manifests in the repository.
Summary
Updates the
smarttaskbarmanifest to use the new upstream repository (Oliviaophia/SmartTaskbar) and restructures the installer logic for maintainability and consistency.Related issues or pull requests
Changes
homepageandlicensefields to reflect the new repositorydescriptionfor better clarity and style consistencyadd-startup,remove-startup)noteswith additional troubleshooting guidancebin,uninstaller, etc.) for readabilityautoupdateURL to match the new GitHub repositoryNotes
smarttaskbaris stored in$env:LOCALAPPDATA\Chanple, so the previous persistence ofSmartTaskbar.exe.configwas meaningless.Testing
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit
New Features
Chores