skills/github__gh-stack
1
0
Fork 0
mirror of https://github.com/github/gh-stack.git synced 2026-09-14 20:26:28 +08:00
Code Issues Packages Projects Releases Wiki Activity
83 Commits 28 Branches 10 Tags
copilot/investigate-cli-bugs
Commit Graph

4 Commits

Author SHA1 Message Date
Sameen Karim 797c62e9b4 Adapt theme colors to light and dark terminals (#149) 
* Make the TUIs adapt to light and dark terminal backgrounds

The submit, view, and modify TUIs were tuned for dark terminals. On light
or solarized-light backgrounds the result was hard to read and inverted:
primary text used ANSI white (invisible on white), dim chrome used light
grays (too faint), and accents used bright cyan (low contrast) — so
"active" things looked lighter than "disabled" ones.

Introduce a centralized, background-aware color palette and migrate all
three TUIs to it:

- internal/tui/shared/theme.go: a semantic palette of lipgloss.AdaptiveColor
  values (primary/muted/faint text, chrome/border, accent, PR-state colors,
  badge backgrounds, row shade, button, switch). lipgloss resolves the
  light/dark variant per render from the terminal background, which Bubble
  Tea detects at startup; terminals that don't report it fall back to dark,
  preserving the original look.
- Replace every hardcoded ANSI color in shared/, submitview/, and
  modifyview/ with palette roles. The four pre-rendered status icons now
  render at use-time so their adaptive colors resolve correctly. The submit
  markdown preview picks glamour's light or dark style from the detected
  background.
- GH_STACK_THEME=auto|light|dark forces the palette for terminals that
  mis-detect (some SSH/tmux setups); wired via the root command's
  PersistentPreRun before any render. Documented in the README and CLI docs.

Neutral text/chrome use truecolor hex (GitHub Primer-inspired) for
predictability across themes, including solarized which repurposes ANSI
8-15; lipgloss downsamples on terminals without truecolor.

Tests verify the palette resolves differently for light vs dark and that
GH_STACK_THEME is honored.

* Apply background-aware colors to all command output

Background detection and the GH_STACK_THEME override (added for the TUIs)
only affected the interactive screens. Plain command output -- status
messages and interactive prompts -- went through the mgutz/ansi library
with fixed ANSI palette names (green/red/yellow/cyan/...), so it never
adapted to the terminal background and could read poorly on light or
solarized themes.

Unify everything on the same adaptive palette so all colors react to the
detected background and to GH_STACK_THEME.

- Extract internal/theme, a foundational package with no internal
  dependencies, that owns:
    - the background-aware lipgloss.AdaptiveColor palette (moved out of
      internal/tui/shared),
    - ApplyOverride(), the GH_STACK_THEME=auto|light|dark logic, and
    - non-TUI colorizers (Success/Error/Warning/Blue/Magenta/Cyan/Gray/
      Bold) plus FgSeqs(), which returns the raw start/reset escapes used
      to color the user's echoed prompt input.
- internal/tui/shared/theme.go now re-exports the palette, so the TUI code
  keeps referring to shared.ColorX unchanged.
- internal/config/config.go wires the Config.Color* funcs to the theme
  colorizers and drops mgutz/ansi (now an indirect dependency only).
- cmd/utils.go colors the prompt icon and echoed input via theme.
- cmd/root.go calls theme.ApplyOverride() in PersistentPreRun.

Detection adds no cost: because the command package imports Bubble Tea,
its init() already triggers (and caches) the terminal background query for
every command, so the non-TUI colorizers just read the cached value.
Terminals that don't answer the query fall back to the dark palette;
GH_STACK_THEME=light|dark forces it. Colors are truecolor on capable
terminals and downsample to the nearest ANSI color elsewhere.

Tests: internal/theme covers palette adaptiveness, ApplyOverride, the
colorizers, and FgSeqs; a new internal/config test verifies the wired-up
Config.Color* funcs adapt to the background when color is enabled.

Docs: README and the CLI reference note that GH_STACK_THEME now controls
all colored output, not just the interactive screens.

No behavior change beyond colors.
 2026-06-29 20:11:09 -04:00
Sameen Karim 0efc764f8c Redesign TUI shared header and embed the GitHub logo as an image (#143) 
* Redesign the shared header and embed the GitHub logo as an image

Rework the shared gh-stack header (used by `view` and `modify`, and reused
by `submit` later in this stack) for a cleaner, more responsive look, and
replace the braille/ASCII Invertocat with a real image of the GitHub mark.

- Logo: embed the Invertocat PNG with go:embed and draw it via an inline-
  image protocol (kitty or iTerm2). It is image-or-nothing: when no protocol
  is available, stdout is not a TTY, or we are inside tmux/screen, no logo is
  drawn and the text falls back to the normal left padding. Detection is
  environment-based and cached so it never blocks the TUI, and a fixed kitty
  image id lets the header clear or replace the logo in place instead of
  leaving copies behind.
- Layout: place the logo in the top-left corner beside the title and version,
  with the stack-info lines left-aligned beneath it on the same left margin.
  Size the box to its content for each view so there is no trailing empty row.
- Responsiveness: hide the logo progressively — first when the viewport is too
  narrow, then a little before the rest of the header at short heights, where
  a vertical resize could otherwise leave a ghost of the inline image.
- Add unit tests for the header's responsive thresholds.

* Drop the unused HeaderHeight constant and dedupe the header config build

Follow-up to review feedback on the shared-header redesign:

- Remove the HeaderHeight constant. Nothing referenced it (callers compute
  height via HeaderHeightFor), and its doc described a "maximum" that
  HeaderHeightFor does not actually enforce, so the comment was misleading.
- In the view and modify View() methods, build the header config once and reuse
  it for both RenderHeader and the height reservation instead of rebuilding it
  twice per frame. The click/scroll handlers keep deriving the height from the
  same config, so the header's dimensions remain a single source of truth.
 2026-06-29 20:11:08 -04:00
Sameen Karim 49a753708c insert branches with modify (#107) 
* add insert branch operation to modify TUI

Add `i` (insert below) and `I` (insert above) key bindings to the
interactive modify view, allowing users to insert new empty branches
into an existing stack. This follows Vim-inspired semantics where
lowercase `i` inserts below the cursor and uppercase `I` inserts above.

## TUI behavior

When the user presses `i` or `I`, the TUI enters an insert input mode
(similar to rename mode) where they type a new branch name. The input
is validated against git ref naming rules, local branch uniqueness, and
in-stack name collisions. On confirm, a placeholder node is inserted at
the correct position in the branch list with a green "✚ insert"
annotation badge and green connector styling.

Insert is a structure operation — it works alongside fold, rename, and
drop, but is mutually exclusive with reorder (consistent with existing
mode exclusivity rules). Undo (`z`) removes the inserted node cleanly.

## Apply engine

At apply time (Step 2 in the pipeline, between renames and folds), the
engine creates the new git branch at the parent branch's tip via
`git.CreateBranch` and inserts a `BranchRef` into the stack metadata at
the correct position. If the insertion changes the base of a branch
that has an open PR, `affectsPRs` is set to trigger a required
`gh stack submit` afterward.

## Header shortcut updates

- Combined the fold shortcuts into a single line: `d/u - fold down/up`
- Added insert shortcuts on their own line: `i/I - insert below/above`
- Reordered fold references throughout to list "down" before "up" for
  consistency with the insert shortcut ordering

## Files changed

- types.go: ActionInsertBelow/ActionInsertAbove types, IsInserted field,
  InsertedBranches in ApplyResult
- model.go: key bindings, insert input mode, undo, mode exclusivity,
  annotation, styling, header shortcuts, effective-index tracking to
  prevent false reorder detection when inserts shift node positions
- styles.go: green insert badge/branch/connector styles
- status.go: insert counting in pending change summary
- help.go: new "Insert below / above" section, reordered fold heading
- apply.go: BuildPlan and ApplyPlan handle insert actions
- modify.go: updated command description and success summary
- README.md: updated keybindings table

## Test coverage

- 16 new TUI tests: insert below/above, top/bottom edges, undo, mode
  exclusivity, merged branch guard, cancel/empty input, duplicate name
  validation, pending summary counting, annotation rendering, mixed
  operations with drop/fold, apply acceptance
- 4 new apply tests: BuildPlan produces correct insert actions,
  ApplyPlan creates branches and updates stack metadata, insert at
  stack start uses trunk as parent, affectsPRs triggered when inserting
  before a branch with an open PR

* update add error msg to direct users to modify for inserting branches

* docs updates

* fix insert branch bugs in modify TUI

Fix three bugs with the insert branch feature in the modify TUI, and
adjust rename behavior on inserted nodes.

## Bug 1: False "moved" annotations on existing branches

After inserting a branch, all branches below the insertion point
displayed "↕ moved 1 layer down" annotations. This happened because
`nodeAnnotation` and `toNodeData` compared each node's
`OriginalPosition` against its raw array index, which gets shifted
when an inserted node is added to the slice.

Fix: introduce an `effectiveIdx` parameter that counts only
non-inserted nodes, so position comparisons reflect the original
ordering. The View loop computes effective indices by incrementing
only for non-inserted nodes and passes them to the rendering
functions.

## Bug 2: Header branch count inflated by staged inserts

The branch count in the header ("N branches") included inserted
placeholder nodes, making it appear as though the stack had grown
before changes were applied.

Fix: `buildHeaderConfig` now excludes `IsInserted` nodes from the
branch count. The count reflects only the original branches in the
stack.

## Bug 3: Operations allowed on inserted placeholder nodes

Inserted nodes could be folded into other branches, which makes no
sense for a placeholder with no commits. Additionally, the "last
branch" guard counted inserted nodes as active, allowing users to
drop all original branches and bypass the empty-stack check.

Fix:
- `fold()` rejects inserted nodes with a descriptive error message.
- `toggleDrop()` on an inserted node removes it entirely and pops
  the original insert action from the undo stack (clean cancellation
  rather than a separate undo entry).
- All three "active branch" guards (`toggleDrop`, `fold`, `tryApply`)
  now exclude `IsInserted` nodes, ensuring at least one original
  branch always remains in the stack.

## Rename on inserted branches

Instead of blocking renames on inserted nodes, pressing `r` now
enters rename mode and updates the insert action's name in place.
The node's `Ref.Branch` and `PendingAction.NewName` are both updated
directly — no separate rename action is created in the undo stack.
This lets users fix a typo without having to drop and re-insert.

## Tests added

- `TestInsertDoesNotShowMovedAnnotation` — verifies no false move
  annotations appear on existing branches after an insert
- `TestBranchCountExcludesInserts` — verifies header count stays
  stable after insert
- `TestCannotFoldInsertedBranch` — verifies fold is blocked
- `TestCannotRenameInsertedBranch` — verifies rename updates the
  insert name in place
- `TestDropInsertedBranchRemovesIt` — verifies drop removes the node
- `TestDropInsertedBranchCanBeUndone` — verifies drop pops the
  original insert from the undo stack
- `TestCannotDropAllOriginalBranchesWithInsert` — verifies the
  empty-stack guard excludes inserted nodes

* ensure cannot fold into an inserted branch

* rm dead code

* delete inserted branches during abort
 2026-05-26 17:39:38 -04:00
Sameen Karim 7a268fc380 modify command (#72) 
* git primitives for modify cmd

* extract reusable TUI parts

* modify cmd

* recreate stack after modify

* add checks to prevent other modifications while modify is applying

* modify continue for resuming after resolving conflicts

* fix bug with duplicate stack entries after modifying

* reuse conflict resolution help msg from rebase

* additional confirmation before overwriting stack on remote

* fix recreate order of operations

Co-authored-by: Copilot <copilot@github.com>

* move base commit instead of cherry picking for fold up

* check to ensure we aren't left with zero branches

* unify and dedupe across view and modify tui

* more detailed help instructions

Co-authored-by: Copilot <copilot@github.com>

* only recommend submit if stack exists on remote

Co-authored-by: Copilot <copilot@github.com>

* tests for modify tui, apply modifications, submit modifications

* refactor submit for regular and pending modifications

* rename recover to abort

Co-authored-by: Copilot <copilot@github.com>

* docs for modify cmd

* tui styling updates

* updated tui screenshot

* addressing review comments

* Fix 4 bugs from code review

Bug 1: Move RevParseMap error check before using originalRefs.
The error from git.RevParseMap() was deferred past iteration of
originalRefs, which could panic on a nil map.

Bug 2: Differentiate cherry-pick vs rebase conflicts in modify.
Cherry-pick conflicts don't save state as 'conflict' phase, so
--continue won't work. Now prints --abort-only instructions for
cherry-pick conflicts.

Bug 3: Unwind now cleans up branches created by renames.
After restoring snapshot branches, Unwind deletes renamed branch
names that don't belong to the original snapshot.

Bug 4: Simplify push message in submit command.
Changed from 'Pushing N branches to remote...' to 'Pushing to
remote...' since individual branches may fail.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix 7 nit issues from code review

11: Add named constants for phase strings (PhaseApplying, PhaseConflict,
PhasePendingSubmit) in state.go; replace remaining raw literals in
state.go CheckStateGuard.

14: Fix bottomLines comment mismatch — listed 3 items but value is 2.

15: Extract magic number 88 to MinWidthForArt constant in header.go.

16: Remove unused stackview import anchor in model.go — the import
is used via types.go where BranchNode is embedded.

17: Simplify CheckStackLinearity parent resolution — ActiveBaseBranch
already handles skipping merged branches.

18: Fix rename undo matching any rename — add NewName check so only
the specific rename being undone is matched.

20: Add TestUndoRename and TestUndoRename_DoesNotAffectOtherRenames
to validate rename undo behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Make cherry-pick conflicts recoverable via --continue

Previously, cherry-pick conflicts during fold-down operations could only
be resolved with --abort. Now they save full conflict state (phase,
conflict type, fold branch/target, remaining branches) to the state file,
enabling recovery via 'gh stack modify --continue'.

Changes:
- Add ConflictType field to StateFile (rebase or cherry_pick)
- Add FoldBranch/FoldTarget fields for cherry-pick context
- Add CherryPickContinue to git package (cherry-pick --continue)
- Save cherry-pick conflict state in ApplyPlan with remaining branches
- ContinueApply handles both rebase and cherry-pick conflicts
- Unified conflict messaging in cmd/modify.go (both types show --continue)
- Updated test to verify cherry-pick conflict state is saved correctly

* Apply suggestions from code review

Co-authored-by: Luke Ghenco <lukeghenco@github.com>
Co-authored-by: Sameen Karim <skarim@github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Luke Ghenco <lukeghenco@github.com>
 2026-05-04 22:34:42 -04:00