Skip to content

Adopt new colorization ergonomics for cli#1037

Merged
0xfornax merged 12 commits intorocket-pool:masterfrom
jshufro:jms/color
Mar 9, 2026
Merged

Adopt new colorization ergonomics for cli#1037
0xfornax merged 12 commits intorocket-pool:masterfrom
jshufro:jms/color

Conversation

@jshufro
Copy link
Contributor

@jshufro jshufro commented Mar 6, 2026

Draft until #1034 lands

@jshufro jshufro force-pushed the jms/color branch 4 times, most recently from b412219 to ed975fd Compare March 6, 2026 23:17
@jshufro jshufro marked this pull request as ready for review March 9, 2026 12:56
@0xfornax
Copy link
Member

0xfornax commented Mar 9, 2026

Bug 1: GreenPrintln called with format args (wrong function)

File: rocketpool-cli/claims/claim-all.go, line ~322

// Old
fmt.Printf("  %sSuccessfully distributed balance of minipool %s.%s\n", colorGreen, mp.Address.Hex(), colorReset)

// New (BUG)
color.GreenPrintln("Successfully distributed balance of minipool %s.", mp.Address.Hex())

GreenPrintln takes ...string — it doesn't accept format specifiers. The %s will be printed literally, and mp.Address.Hex() will be ignored entirely. Should be color.GreenPrintf(...).


Bug 2: GreenSprintf result is discarded (silent no-op)

File: rocketpool-cli/service/service.go, printPatchNotes function

// Old
fmt.Printf("%s=== Smart Node v%s ===%s\n", colorGreen, shared.RocketPoolVersion(), colorReset)

// New (BUG)
color.GreenSprintf("=== Smart Node v%s ===\n", shared.RocketPoolVersion())

GreenSprintf returns a string — it doesn't print anything. The version header is silently dropped and never displayed.


Summary

# File Severity Description
1 claims/claim-all.go 🔴 High GreenPrintln used with format args — address is silently dropped from output
2 service/service.go 🔴 High GreenSprintf return value discarded — version banner never printed

@0xfornax 0xfornax merged commit 38b6708 into rocket-pool:master Mar 9, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants