fix(scoop-info): improve installed size display to show app + user data combined - #6651
fix(scoop-info): improve installed size display to show app + user data combined#6651B67687 wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdds aligned installed-size formatting helpers, uses them unconditionally in verbose scoop-info output, suppresses filesystem enumeration errors during recursive size summation, adds Pester tests for the formatter, and updates CHANGELOG.md. ChangesInstalled size output enhancement
Sequence Diagram(s)sequenceDiagram
participant scoop-info
participant format_installed_size
participant console_output
scoop-info->>format_installed_size: format_installed_size(current,persisted,cache,old)
format_installed_size->>console_output: formatted lines (Application, Old versions?, Download cache?, Total)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
|
This is more opinionated lol but what do yall think |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CHANGELOG.md (1)
5-5: 💤 Low valueConsider a more descriptive changelog entry.
The current entry mentions combining sizes but doesn't capture all the changes mentioned in the PR summary: renaming "Cached downloads" to "Download cache", the parenthetical breakdown format, or the
-ErrorAction SilentlyContinueaddition. Consider expanding to:- **scoop-info:** Clarify verbose installed size output by combining application files and persisted data into a single "Application" line with breakdown, rename "Cached downloads" to "Download cache", and suppress filesystem errors ([`#4840`](https://github.com/ScoopInstaller/Scoop/issues/4840))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` at line 5, Update the CHANGELOG.md entry for scoop-info to the more descriptive line: state that verbose installed size now combines application files and persisted data into a single "Application" line with a parenthetical breakdown, that "Cached downloads" was renamed to "Download cache", and that filesystem errors are suppressed via -ErrorAction SilentlyContinue; reference the PR number (`#4840`) in the entry as shown in the suggested text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 302: Replace the truncated placeholder entry `- **chore:** Upda...
trimmed for brevity` in the Bug Fixes section for v0.2.3 with the full, original
changelog line; locate the entry by searching for the v0.2.3 heading and the
exact placeholder string, then restore the complete original text from the
repository history or backup (or the author’s original commit message) so the
changelog contains the full, valid changelog entry instead of the trimmed
placeholder.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Line 5: Update the CHANGELOG.md entry for scoop-info to the more descriptive
line: state that verbose installed size now combines application files and
persisted data into a single "Application" line with a parenthetical breakdown,
that "Cached downloads" was renamed to "Download cache", and that filesystem
errors are suppressed via -ErrorAction SilentlyContinue; reference the PR number
(`#4840`) in the entry as shown in the suggested text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 1-7: Remove the duplicate "## [Unreleased]" heading by keeping a
single "## [Unreleased]" section, then move the bug-fix bullet (the scoop-info
bullet about "Clarify verbose installed size...") so it appears under the
retained "### Bug Fixes" list; ensure there is only one "## [Unreleased]"
heading and one "### Bug Fixes" block, and delete the redundant trailing
duplicated heading and any blank lines so the changelog markdown is well-formed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libexec/scoop-info.ps1 (1)
177-200: ⚡ Quick winAdd a regression test for the new verbose installed-size layout.
This block now encodes multiple presentation and arithmetic rules (Application merge/breakdown, conditional rows, label rename, unchanged Total), so a focused output regression test would prevent drift.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libexec/scoop-info.ps1` around lines 177 - 200, Add a regression test that validates the verbose "Installed size" output produced from the $fileTotals -> $fileSizeOutput logic: exercise cases for (a) typical app with app+user data (ensure "Application: " line shows combined size and the "(X app + Y user data)" breakdown), (b) when old versions exist (ensure "Old versions: " appears only if $fileTotals[4] != 0), (c) when download cache exists (ensure "Download cache: " appears only if $fileTotals[3] != 0), and (d) Total calculation remains unchanged (verify "Total: " equals filesize($fileTotals[0] + $fileTotals[2] + $fileTotals[3])). Use the same formatting helpers (filesize, PadLeft, and newline join that populates $item.'Installed size') and assert the full multi-line string exactly to catch presentation or arithmetic regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libexec/scoop-info.ps1`:
- Around line 177-200: Add a regression test that validates the verbose
"Installed size" output produced from the $fileTotals -> $fileSizeOutput logic:
exercise cases for (a) typical app with app+user data (ensure "Application: "
line shows combined size and the "(X app + Y user data)" breakdown), (b) when
old versions exist (ensure "Old versions: " appears only if $fileTotals[4] !=
0), (c) when download cache exists (ensure "Download cache: " appears only if
$fileTotals[3] != 0), and (d) Total calculation remains unchanged (verify
"Total: " equals filesize($fileTotals[0] + $fileTotals[2] +
$fileTotals[3])). Use the same formatting helpers (filesize, PadLeft, and
newline join that populates $item.'Installed size') and assert the full
multi-line string exactly to catch presentation or arithmetic regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 24d861d5-6979-44a7-b3c5-fceaf81fd918
📒 Files selected for processing (1)
libexec/scoop-info.ps1
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/core.ps1`:
- Around line 354-365: The "Application" line always appends the "($(filesize
$currentSize) app + $(filesize $persistedSize) user data)" suffix even when
$persistedSize is 0; change the code that builds the Application line (where
$appSize is computed and format_installed_size_line is called) to only include
that suffix when $persistedSize -gt 0, otherwise call format_installed_size_line
with just the app size/label; keep using the existing variables ($currentSize,
$persistedSize, $appSize) and the format_installed_size_line helper so the
output shows "Application: X (Y app + Z user data)" only when persisted data
exists.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1d26f27-9d81-4a1f-a263-4b964bad3e67
📒 Files selected for processing (3)
lib/core.ps1libexec/scoop-info.ps1test/Scoop-Core.Tests.ps1
✅ Files skipped from review due to trivial changes (1)
- test/Scoop-Core.Tests.ps1
…ta combined Improve the 'scoop info -v' installed size display by combining app files and persisted data into a single 'Application' line, reducing confusion about what counts toward the installed footprint. - Combine 'Current version' and 'Persisted data' into 'Application' line - Show breakdown in parentheses: (X app + Y user data) - Rename 'Cached downloads' to 'Download cache' - Right-align size values in a fixed-width column - Add -ErrorAction SilentlyContinue to suppress filesystem errors - Add regression tests for installed-size formatting Closes ScoopInstaller#4840
e659e85 to
081ec4b
Compare
✅ Actions performedReviews resumed. |
Summary
Improve the
scoop info -vinstalled size display by combining app files and persisted data into a singleApplicationline, reducing confusion about what counts toward the installed footprint.Motivation and Context
Users were confused by seeing separate large values for the current version and persisted data, which made it look like Scoop was double-counting storage. Grouping them under a single application total makes the output easier to reason about while still preserving the breakdown.
Relates to #4840.
Changes
Current versionandPersisted datainto a singleApplicationline(X app + Y user data)Cached downloadstoDownload cache-ErrorAction SilentlyContinuewhile enumerating files to avoid noisy failures on unusual file typesBefore
After
After (when old versions are present)
Old versionsis shown only when it is non-zero.How Has This Been Tested?
scoop info -voutput on an app with large persisted dataApplicationlabel and inline breakdown render as expectedOld versionsappears only when present-ErrorAction SilentlyContinuesuppresses errors on non-standard file typesInvoke-Pester -Path .\test\Scoop-Core.Tests.ps1 -ExcludeTagFilter Windows -CIwith Pester 5.7.1Checks
developbranch.Summary by CodeRabbit
Bug Fixes
Tests