Skip to content

feat(charm): add lipgloss styling with Slack brand colors under charm experiment - #365

Merged
srtaalej merged 17 commits into
mainfrom
charm-lipgloss-migration
Mar 11, 2026
Merged

feat(charm): add lipgloss styling with Slack brand colors under charm experiment#365
srtaalej merged 17 commits into
mainfrom
charm-lipgloss-migration

Conversation

@srtaalej

@srtaalejsrtaalej commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Changelog

Adds lipgloss-based styling to all help output under the charm experiment flag, using a centralized Slack brand color palette shared across all CLI components, and migrates all direct Styler() usage to named style functions.

Summary

This PR started as a focused effort to add lipgloss styling to help output, but grew in scope for two main reasons:

  1. Charm v2 upgrade — Upgrading huh to v2 was necessary to fix bugs in the interactive forms related to styling. The v2 API introduced breaking changes to key press handling in tests (create_template_charm_test.go), requiring updates across all huh form test files.
  2. Styler migration — To cleanly support both aurora (legacy) and lipgloss (charm) code paths, all external Styler() call sites were migrated to named style functions (Green(), Red(), Yellow(), Gray()). This touched many files but each change is mechanical.

The PR covers four logical areas:

  • chore(deps): Bump charm packages (huh, lipgloss) to v2
  • refactor: Migrate all Styler() call sites to named style functions
  • feat(experimental): Add Slack brand color palette (colors.go) and lipgloss style implementations
  • feat(experimental): Add charm-styled help page templates with flag colorization

Screenshots

Screenshot 2026-03-10 at 7 24 57 PM

Test steps

make test testdir=internal/style # all tests pass (including new TestStyleFlags and TestStyleFlags_CharmDisabled)
make test testdir=cmd/help # all tests pass
make build
./bin/slack --help -e charm # flags show yellow names + gray descriptions, alias arrow uses ❱
./bin/slack project --help -e charm # subcommand flags also styled (inherited help func)
./bin/slack run --help -e charm # custom run help also styled (uses PrintHelpTemplate)
./bin/slack --help (no experiment) # flags unchanged (plain text), arrow stays ># Verify on both light and dark terminal backgrounds if possible

Known issues (follow-up)

Per reviewer feedback, these will be addressed in follow-up PRs:

  • Errors shown with green output (confusing to skim)
  • Section headings not contrasted with section text
  • Debug output loses highlighting when charm experiment is active

Code changes (to keep this PR from growing further):

  • Remove IsCharmEnabled() from internal/style and use
    clients.Config.WithExperimentOn(experiment.Charm) checks outside the style package instead
    (help.go:73, style.go:63). Deferred because it also requires changes to
    cmd/platform/run.go.
  • Avoid renaming stable code for experimental changes — consider renaming charmHelpTemplate
    back to helpTemplate when the experiment concludes (help.go:127)
  • Improve create_template_charm_test.go to assert expected values passed to huh forms
    rather than testing prompt interactions directly

Requirements

@srtaalejsrtaalej added this to the Next Release milestone Mar 6, 2026
@srtaalejsrtaalej self-assigned this Mar 6, 2026
@srtaalejsrtaalej added experiment Experimental feature accessed behind the --experiment flag or toggle semver:patch Use on pull requests to describe the release version increment labels Mar 6, 2026
@codecov

codecovBot commented Mar 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.07692% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.49%. Comparing base (9ab82a5) to head (cd2e287).
⚠️ Report is 3 commits behind head on main.

Files with missing linesPatch %Lines
internal/style/template.go8.33%32 Missing and 1 partial ⚠️
internal/pkg/platform/activity.go33.33%4 Missing ⚠️
internal/iostreams/printer.go25.00%1 Missing and 2 partials ⚠️
cmd/help/help.go50.00%1 Missing and 1 partial ⚠️
cmd/manifest/validate.go0.00%2 Missing ⚠️
cmd/upgrade/upgrade.go0.00%2 Missing ⚠️
internal/style/style.go97.75%2 Missing ⚠️
internal/style/format.go96.66%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #365 +/- ##
==========================================
+ Coverage 65.19% 65.49% +0.29% 
==========================================
Files 218 218 Lines 17981 18120 +139 ==========================================
+ Hits 11723 11867 +144 + Misses 5164 5160 -4 + Partials 1094 1093 -1 

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@srtaalej
srtaalej marked this pull request as ready for review March 10, 2026 17:48
@srtaalej
srtaalej requested a review from a team as a code ownerMarch 10, 2026 17:48
@srtaalej
srtaalej marked this pull request as draft March 10, 2026 17:54
@srtaalej
srtaalej marked this pull request as ready for review March 11, 2026 00:02
@mwbrooksmwbrooks added changelog Use on updates to be included in the release notes and removed changelog Use on updates to be included in the release notes labels Mar 11, 2026

@zimegzimeg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srtaalej Thanks for sharing these changes! I'm approving now in moon light so we might want to follow up with adjustments to the theme next 🌚

Some of these comments suggest that this PR should be broken up into perhaps multiple separate changes. No blocker since I think it's working alright but review might be more focused I hope - perhaps as: 🙏

  1. chore(deps): update charm packages to v2
  2. refactor: use style package for all stylings
  3. feat(experimental): update branding and styles
  4. feat(experimental): format help page outputs

Both 1 and 2 LGTM but I want to leave a few comments toward 3 then 4 next:

With a few test runs using the experiment I find that:

  • Errors are shown with green outputs which are confusing to skim I felt 👾
  • Section headings aren't contrasted with section text which make walls of text
  • Debug outputs lose highlighting after the experiment becomes active 🐛

Some related writings I find kind are clig.dev but we might find compromise in amount of formats for preference to theme. FWIW I find colors of the help pages helpful after patterns are gathered! Toward 4 we might want to consider the fang package for included batteries? 🔋 ⚡

Let's get this merged I think! The above can be discussed separate but needn't block experiment 🚢

Comment threadinternal/style/style_test.go
Comment threadcmd/manifest/validate_test.go Outdated
Comment on lines 97 to +104
// Navigate down to "View more samples" (4th option, index 3)
_, cmd := f.Update(tea.KeyMsg{Type: tea.KeyDown})
_, cmd := f.Update(tea.KeyPressMsg{Code: tea.KeyDown})
doAllUpdates(f, cmd)
_, cmd = f.Update(tea.KeyMsg{Type: tea.KeyDown})
_, cmd = f.Update(tea.KeyPressMsg{Code: tea.KeyDown})
doAllUpdates(f, cmd)
_, cmd = f.Update(tea.KeyMsg{Type: tea.KeyDown})
_, cmd = f.Update(tea.KeyPressMsg{Code: tea.KeyDown})
doAllUpdates(f, cmd)
_, cmd = f.Update(tea.KeyMsg{Type: tea.KeyEnter})
_, cmd = f.Update(tea.KeyPressMsg{Code: tea.KeyEnter})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🪬 thought: These changes aren't related to lipgloss which make review evermore difficult but also make me think we're testing too much related to prompts here. I'm not familiar with this implementation but I'd expect assertions that expected values are passed to huh forms?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these were updated to match the new v2 syntax for key presses - i agree this PR got really big really fast. i wanted to update to v2 here to resolve the bugs we had with the huh forms which i felt were related to styling. i just didnt realize there would be so much needing changing 😭

Comment threadcmd/help/help.go
Comment on lines +121 to +127
// ════════════════════════════════════════════════════════════════════════════════
// DEPRECATED: Legacy help template — aurora styling
//
// Delete this entire block when the charm experiment is permanently enabled.
// ════════════════════════════════════════════════════════════════════════════════

const legacyHelpTemplate string = `{{.Long}}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 suggestion: We should avoid changing existing code with experimental changes. I imagine we'll want to rename charmHelpTemplate back to helpTemplate after this experiment concludes, but references to stable code is being changed often during this. I'm not against moving code but it adds more to review!

Comment threadcmd/upgrade/upgrade.go
Comment on lines +1705 to +1710
"non-breaking warning continues without prompt": {
warnings: slackerror.Warnings{
{Code: "some_warning", Message: "just a warning"},
},
expectedResult: true,
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧪 suggestion: While we're adding this we should confirm clients.IO.ConfirmPrompt isn't called in asserts below to match the test case. This might otherwise be prompting a default "no" to confirming changes of the manifest-

Comment threadinternal/style/colors.go
Comment threadinternal/style/colors.go
span, _ := opentracing.StartSpanFromContext(ctx, "printInfo", opentracing.Tag{Key: "printInfo", Value: message})
defer span.Finish()
}
io.Stdout.Println(style.Styler().Reset(message))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👁️‍🗨️ issue: I'm not super confident in this change to production code since this might've been guarding "info" output from being formatted in terminals as bold or secondary from previous lines.

We might continue to support this with exception for the charm experiment? I agree that it's not ideal to support edge cases of earlier output but it might keep experiences most stable.

Comment threadinternal/style/format.go Outdated
@srtaalej
srtaalej merged commit 755ccfe into mainMar 11, 2026
8 checks passed
@srtaalej
srtaalej deleted the charm-lipgloss-migration branch March 11, 2026 17:33
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

experimentExperimental feature accessed behind the --experiment flag or togglesemver:patchUse on pull requests to describe the release version increment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@srtaalej@zimeg@mwbrooks