Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
794d863
fix(review): bound search-jump gap reveal and keep current match
lettertwo Jul 27, 2026
095d6fb
feat(review): copy path:line to clipboard via OSC 52
lettertwo Jul 27, 2026
ecfae80
fix(review): tolerate and self-heal a stale-diff align race
lettertwo Jul 27, 2026
7329d45
docs(review): log cursor-side, zoom-cycle, and deferred-load gaps
lettertwo Jul 27, 2026
8297ef5
docs(review): log combined-view exemptions as zoom evidence
lettertwo Jul 27, 2026
fd653ab
feat(review): split yank into range-aware copy-lines and copy-location
lettertwo Jul 27, 2026
ae7dc96
fix(review): keep selection when the clipboard write fails
lettertwo Jul 27, 2026
509abb4
docs(review): drop stale detect_gt note from pty bound
lettertwo Jul 29, 2026
d47974b
docs(review): record the focus-maximize view state decision
lettertwo Aug 10, 2026
b40b138
refactor(review)!: replace the zoom enum with focus and maximize
lettertwo Aug 10, 2026
c00ab4b
fix(review): reload the index before reading a staged blob
lettertwo Aug 30, 2026
4c739f7
docs(review): note the loader handle's stale index in ADR-037
lettertwo Aug 30, 2026
dc2dc27
feat(review): name the role in a single staged or unstaged pane
lettertwo Aug 30, 2026
0d93e60
feat(review): hide the staging hints where staging can only refuse
lettertwo Aug 31, 2026
001d6a3
refactor(review): rename Role::Combined to Role::Whole
lettertwo Sep 1, 2026
a5bfbde
docs(review): name milestones, traps, and changesets instead of numbe…
lettertwo Sep 1, 2026
9484d94
docs(review): replace milestone, trap, and changeset labels in source…
lettertwo Sep 1, 2026
e733088
docs(lib): replace milestone labels in stack and changeset comments
lettertwo Sep 1, 2026
9381816
docs(cli): replace milestone labels in dispatch and completer comments
lettertwo Sep 1, 2026
9603593
test(review): rename tests that carry trap and milestone labels
lettertwo Sep 1, 2026
6e41f76
style(review): rewrap swept comments to 100 columns
lettertwo Sep 1, 2026
c55f09b
docs(review): replace milestone, trap, and changeset labels in app.rs
lettertwo Sep 1, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/INDEX.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ Maps subsystems and topics to relevant documentation files and source paths. Use

- `docs/rfc/workon-review.md`
- `docs/adr/033-review-crate-workspace-placement.md`
- `docs/adr/038-review-focus-maximize-replaces-zoom.md` — the diff pane's view state, and why `Role::Whole` survives
- Key source: `git-workon-review/src/`

## Cross-cutting Concerns
Expand Down
8 changes: 4 additions & 4 deletions docs/adr/033-review-crate-workspace-placement.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@ The RFC (`docs/rfc/workon-review.md`) defines `git-workon-review`, a standalone
Add `git-workon-review` as a sibling crate: lib target `workon_review`, bin target `git-workon-review`.

- **`publish = false`** in the crate's `Cargo.toml` is the single knob that keeps it out of both release-plz and cargo-dist, following the `git-workon-fixture` precedent (proven across ~20 releases).
- **`[package.metadata.dist] dist = false`** is set explicitly as well. It is redundant today (`publish = false` already excludes the crate) but is the tripwire for the M3 flip: removing `publish = false` alone, without also deciding this field, would silently make cargo-dist ship the binary.
- **`[package.metadata.dist] dist = false`** is set explicitly as well. It is redundant today (`publish = false` already excludes the crate) but is the tripwire for the initial-renderer flip: removing `publish = false` alone, without also deciding this field, would silently make cargo-dist ship the binary.
- **Independent versioning**: the crate does not join `version_group = "main"` in `release-plz.toml`. Joining would lockstep its version to the CLI's and cross-bump the CLI on every review-crate change.
- **Workspace `rust-version` bumped to `1.88`** (ratatui 0.30's floor). `clap` 4.6 already required 1.85, so the workspace's previous `1.68.2` declaration was already unsatisfiable in practice; only the fixture crate had ever inherited the field. `rust-version.workspace = true` is added to all four crates so the field is real everywhere.

## Consequences

- The crate builds and tests in CI from the start (M0) without appearing in crates.io or in any cargo-dist release artifact.
- The M3 flip (when the review binary is ready to distribute) requires:
- The crate builds and tests in CI from the start (crate scaffolding) without appearing in crates.io or in any cargo-dist release artifact.
- The initial-renderer flip (when the review binary is ready to distribute) requires:
1. Remove `publish = false` from `git-workon-review/Cargo.toml`.
2. Add `[[package]] name = "git-workon-review"` to `release-plz.toml`, with **no** `version_group` — independent versioning is intentional, not an oversight to fix later.
3. Keep `dist = false` until binary distribution is designed. The homebrew publish job in `.github/workflows/release.yml` patches **every** `Formula/*.rb` with `git-workon`'s man page and completions install lines; it must be reworked before a second binary can safely flow through it. `release-plz.yml`'s `dist` dispatch step is also hardcoded to fire only for `package_name == "git-workon"` and needs updating too.
- Until the M3 flip, the crate's version in its own `Cargo.toml` is cosmetic — release-plz never touches it.
- Until the initial-renderer flip, the crate's version in its own `Cargo.toml` is cosmetic — release-plz never touches it.

## References

Expand Down
16 changes: 8 additions & 8 deletions docs/adr/034-review-git-native-config-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
## Context

The review TUI (`git-workon-review`) grew its keybindings and colors as hardcoded
values during M3–M5: a `match` in `tui.rs` for keys, a block of `const … Color::Rgb(…)`
values during the initial-renderer-through-stack-and-outline work: a `match` in `tui.rs` for keys, a block of `const … Color::Rgb(…)`
atop `render.rs` for theming. Making either user-configurable needs a config home, and
the review binary reads no config today (`struct Cli {}` is empty).

Expand Down Expand Up @@ -32,8 +32,8 @@ is stored **action-as-key** in **per-view subsections**:

```
workon.review.theme = dark ; global, non-view
workon.review.theme.<slot> = #rrggbb ; base00-base0f override (CS1)
workon.review.theme.<tint> = #rrggbb ; diff/cursor tint override (CS1)
workon.review.theme.<slot> = #rrggbb ; base00-base0f override (git-config reader)
workon.review.theme.<tint> = #rrggbb ; diff/cursor tint override (git-config reader)
workon.review.<view>.bind.<action> = "<key tokens>" ; a keymap entry
workon.review.<view>.<setting> = <value> ; view config
```
Expand All @@ -60,8 +60,8 @@ workon.review.<view>.<setting> = <value> ; view config
- **View config** (non-binding) shares the view namespace: `workon.review.outline.width`,
`workon.review.outline.mode`, `workon.review.diff.layout`, `workon.review.diff.zoom`.
The `.bind.` marker is what distinguishes a keymap entry from a view setting.
- **Theme overrides** (CS1, user-configurable colors tier — see
[ADR-035](035-review-theming-base16-hybrid.md)'s CS1 revision) live in the `review.theme`
- **Theme overrides** (the git-config reader, user-configurable colors tier — see
[ADR-035](035-review-theming-base16-hybrid.md)'s git-config-reader revision) live in the `review.theme`
subsection, distinct from the top-level `workon.review.theme` selection itself: `workon.review
.theme.base00`–`workon.review.theme.base0f` (base16 slot overrides) and eleven kebab-case tint
keys (`workon.review.theme.cursor-bg`, …). Same validation posture as an unknown bind
Expand Down Expand Up @@ -100,8 +100,8 @@ workon.review.<view>.<setting> = <value> ; view config
- Like all git-native config (ADR-006), review config is **not checked into the repo**, so a
team cannot ship a shared review keymap/theme in-tree. Accepted: this is a
personal-productivity TUI.
- The per-view namespace gives previously-hardcoded view settings (outline width — M5
deferred narrow-terminal handling — outline mode, diff layout/zoom defaults) a natural
- The per-view namespace gives previously-hardcoded view settings (outline width — the
stack-and-outline work's deferred narrow-terminal handling — outline mode, diff layout/zoom defaults) a natural
home without a second design pass.
- Adding a rebindable action = adding it to the enumerable action set (code default +
dispatch + help entry); it is automatically configurable, validated, and documented.
Expand Down Expand Up @@ -148,6 +148,6 @@ enough, since the overwhelmingly common cause of an unknown key is a typo of a r
## References

- [ADR-006](006-git-native-config.md) — git-native config under `workon.*` this extends
- `docs/rfc/workon-review.md` — RFC; this is the everyday-usability pass inserted ahead of M7
- `docs/rfc/workon-review.md` — RFC; this is the everyday-usability pass inserted ahead of the source-selector work
- `git-workon-review/src/tui.rs` — current hardcoded keymap (`map_key`) being replaced
- `git-workon-review/src/render.rs` — current hardcoded palette (`const … Color::Rgb`) — see the theming decision
43 changes: 23 additions & 20 deletions docs/adr/035-review-theming-base16-hybrid.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

## Context

The review TUI's colors were hardcoded during M3–M5: a `const … Color::Rgb(…)` block
The review TUI's colors were hardcoded during the initial-renderer-through-stack-and-outline work: a `const … Color::Rgb(…)` block
atop `render.rs` (dark-only) and a parallel `HIGHLIGHT_NAMES`/`HIGHLIGHT_COLORS` pair in
`highlight.rs`. The everyday-usability pass (ahead of M7, see [ADR-034](034-review-git-native-config-schema.md))
`highlight.rs`. The everyday-usability pass (ahead of the source-selector work, see [ADR-034](034-review-git-native-config-schema.md))
adds built-in light/dark theming and terminal adaptivity. Four things had to be resolved:
the color *philosophy* (respect the terminal's 16 ANSI colors vs. ship tuned truecolor),
the theme *primitive*, the *mechanism* by which a theme reaches syntax highlighting, and
Expand All @@ -30,7 +30,7 @@ is spec-conformant.
variants, cursor, selection, and **syntax**. Contrast is guaranteed because foreground and
background come from the *same* scheme.
- **Chrome (default text, dim labels, gutter/dividers) + the canvas background →
base16-ramp-controlled (revised post-CS6):** originally these were ANSI-named
base16-ramp-controlled (revised after the terminal-derivation-probe work):** originally these were ANSI-named
(`Color::Gray`/`DarkGray`) and the canvas was never painted, on the theory that inheriting
the terminal's own bg/fg would self-adapt for free. In practice this broke explicit
`light`/`dark` selections outright — the terminal's own (often dark) bg/fg bled straight
Expand All @@ -51,17 +51,20 @@ capture→slot template.

Diff-bg tints ideally come from base08 (red / spec "Diff Deleted") and base0B (green / spec
"Diff Inserted") and the scheme background, so syntax and tints stay coordinated. **But the
derivation is luminance-dependent, not a single "blend toward base00" (corrected in CS4):**
- **Dark (base00 dark):** the shipped M3–M5 tints are more saturated/darker than *any* convex
derivation is luminance-dependent, not a single "blend toward base00" (corrected in the
base16-palette-primitive work):**
- **Dark (base00 dark):** the shipped initial-renderer-through-stack-and-outline tints are more saturated/darker than *any* convex
blend of an accent toward a dark base00 can produce (their green/blue channels sit *below*
base00's). A blend toward a dark base00 also yields muddy mid-tones, not punchy washes. So
the **dark tints are held explicit** in `Palette::dark()` (byte-identical to M3–M5, per the
the **dark tints are held explicit** in `Palette::dark()` (byte-identical to the
initial-renderer-through-stack-and-outline values, per the
pixel-identity gate). Deriving them would require scaling the accent toward *black* plus a
desaturation step, not a base00 blend — not worth reverse-engineering the hand-tuned values.
- **Light (base00 light) and terminal-derived:** blending an accent toward a *light* base00
gives the correct pale tint, so the `tint_toward` derivation applies there (CS5/CS6). A
gives the correct pale tint, so the `tint_toward` derivation applies there (the
curated-light-scheme and terminal-derivation-probe work). A
terminal-derived theme on a *dark* background hits the same problem as dark and needs the
toward-black+desaturate construction — a CS6 concern.
toward-black+desaturate construction — a terminal-derivation-probe concern.

Net: the scheme-coordinated derivation is real but must branch on background luminance; dark
stays authored.
Expand Down Expand Up @@ -100,7 +103,7 @@ stays authored.
the curated scheme chosen by background luminance if `OSC 11` answered, else `dark`.
tmux/screen/ssh non-response is handled by the timeout, never a hang.

**CS6 refinement — the diff-bg tints stay curated, only the scheme is derived.** In
**Terminal-derivation-probe refinement — the diff-bg tints stay curated, only the scheme is derived.** In
implementation, `auto` derives the base16 **scheme** (the 16 slots → syntax + monochrome ramp)
from the terminal, but the **diff/cursor/selection tints stay curated by luminance** rather than
derived from the probed accents (`Palette::from_terminal`: syntax = `SYNTAX_SLOTS` over the probed
Expand All @@ -117,7 +120,7 @@ ANSI-less slots are still synthesized as above; `parse` → `build_base16` → `
read left untested (see `terminal_query.rs`).

**Derived-washes addendum (2026-07-20) — `auto`'s diff washes derive from the probed accents
after all; cursor/selection washes stay curated.** The CS6 refinement above is partially
after all; cursor/selection washes stay curated.** The terminal-derivation-probe refinement above is partially
reversed. Its objection (1) — "a convex blend toward a dark base00 can't reproduce the
hand-tuned washes" — turned out to answer the wrong question: the goal isn't to reproduce the
curated washes from probed inputs, it's to produce the washes the terminal's *theme author*
Expand All @@ -127,7 +130,7 @@ computes its editor diff backgrounds as `accent:mix(bg, 90)` — exactly the
`tint_toward(accent, bg, k)` shape. `Palette::from_terminal` now derives del washes from probed
base08 and add washes from probed base0B toward the probed base00: a dark probed background uses
the dogfood-validated ratios (subtle 0.90, strong 0.75, staged 0.94/0.85 — staged still reads
dimmer, locked decision #7), a light one reuses `Palette::light`'s hand-tuned ratio set.
dimmer, per the staging-verbs staged-attribution decision), a light one reuses `Palette::light`'s hand-tuned ratio set.
Objection (2) — arbitrary-palette unpredictability — is accepted residual risk, bounded by the
`workon.review.theme.*` override tier (a wash that derives badly on some exotic palette is
pinnable per-user). Cursor/selection/unfocused washes keep borrowing the curated set: they have
Expand All @@ -150,7 +153,7 @@ surprises (a teal cursor row on an aqua-leaning theme), so that judgment stays c
to render; `FgSpan` loses its `Color` field in favor of a capture index. Existing render
tests that assert concrete colors must resolve through a fixed test `Palette`.

## Revised (CS2, visual-polish pass)
## Revised (promoting semantic foregrounds to palette knobs, visual-polish pass)

The "chrome that is never a theme knob (error/warn/current-marker) stays ANSI/const in
`render.rs`" clause above is superseded. Those three colors are now `Palette` fields
Expand All @@ -162,7 +165,7 @@ precedent the diff/cursor tints follow); `light()` takes `ONE_LIGHT`'s base08/ba
the syntax slots (matching the terminal, not curated-tint-borrowing). No other part of the
hybrid boundary changes: this only moves three named colors from `const` to palette fields.

## Revised (CS1, user-configurable colors tier)
## Revised (user-configurable color-override keys)

The "user-supplied base16 scheme … the deferred 'user-configurable colors' tier" noted in
Consequences above lands, narrower than originally sketched: **per-slot and per-tint git-config
Expand Down Expand Up @@ -191,7 +194,7 @@ resolved — the mechanism is base-agnostic, so an override key works identicall
role-mapped field(s) even when the current base hand-authored that field explicitly (e.g.
`base08` under `theme = dark` replaces `dark()`'s hand-tuned `error_fg`) — the alternative
(silently ignoring slot overrides for authored fields) is a UX trap: a user who sets `base08`
expects red to change. Slot overrides do NOT re-derive the diff/cursor tints; that stays the 11
expects red to change. Slot overrides do NOT re-derive the diff/cursor tints; that stays the 12
tint keys' job, applied last and verbatim, so a slot override can't reshape a hand-tuned wash it
wasn't asked to touch. An invalid value or an unrecognized key under `workon.review.theme.*` is
ignored with a startup warning (the same posture as ADR-034's keybinding validation) — not a
Expand All @@ -203,7 +206,7 @@ to vendor) was considered and set aside — the override-key tier covers the imm
named schemes slot in additively later (a `Theme::Named` variant + a `schemes.rs` of vendored
constants) without touching this work if demand appears.

**NO_COLOR (CS2).** The other extra this tier's Context section named — `NO_COLOR`, no CLI flag,
**NO_COLOR (monochrome rendering).** The other extra this tier's Context section named — `NO_COLOR`, no CLI flag,
no color-depth downgrade — lands as `Palette::mono(light: bool)`: every fg field, every syntax
entry, and the canvas background collapse to `Color::Reset` (`paint_canvas: false`), while the
11 diff/cursor washes become achromatic grayscale `Rgb` ladders (dark-terminal vs light-terminal
Expand All @@ -225,9 +228,9 @@ a palette-EXTERNAL color source `mono()`'s own `Color::Reset` fields can't reach
carries a `colorless` flag (`false` on every curated/probed constructor, `true` only on `mono`)
and `render.rs`'s icon paint sites collapse to `foreground` themselves whenever it's set.

## Revised (CS11, diff foreground/background split)
## Revised (diff foreground/background split)

CS1's override table above named the diff washes `subtle`/`strong`. That naming is retired: it
The user-configurable-color-override-keys work's override table above named the diff washes `subtle`/`strong`. That naming is retired: it
described *intensity*, and intensity names invite reuse wherever something should look emphatic.
`render.rs`'s outline status column duly reached for `add_strong`/`del_strong` as **foregrounds**
for the X/Y letters — a background wash used as text color. On a theme whose washes are dark (the
Expand Down Expand Up @@ -277,7 +280,7 @@ then has no background signal, and the foreground is dimming against a full-stre
derivation dims by up to the nominal ratio and stops early at a relative-luminance floor against
that state's own edit wash. This is the first real contrast math in `theme.rs`, whose only prior
arithmetic was `tint_toward`'s per-channel lerp; it is worth the ~25 lines because the failure it
prevents is silent and theme-dependent. Note this is *not* the CS1 blend trap — that was about a
prevents is silent and theme-dependent. Note this is *not* the user-configurable-color-override-keys blend trap — that was about a
convex blend being unable to *reproduce* `dark()`'s hand-tuned washes (channels below base00);
blending an accent toward base00 for a foreground is well-defined, and `light()` already does it.

Expand All @@ -288,7 +291,7 @@ text also retints the status column. Accepted; a theme wanting them apart can be
appears.

**`workon.review.diff.text` selects the foreground source on changed lines** — `syntax` (default,
pixel-identical to CS1 behavior), `tint` (changed lines take the tint foreground), `edit` (syntax
pixel-identical to the user-configurable-color-override-keys behavior), `tint` (changed lines take the tint foreground), `edit` (syntax
stays on the line; only edits take the tint foreground). Context lines always keep syntax
highlighting in every mode; `NO_COLOR`/`mono` still wins over all of it, unchanged. In `edit` mode
an unpaired line takes the tint foreground across its full width, preserving the invariant
Expand All @@ -308,7 +311,7 @@ nothing, now documented rather than surprising.
existing unknown-key startup warning. Pre-1.0, and a dual vocabulary would keep the retired model
discoverable — which is the thing this revision exists to undo.

**Corrections to CS1's table above:** it says "the 11 tint keys" while listing 12, and omits
**Corrections to the user-configurable-color-override-keys table above:** it said "the 11 tint keys" while listing 12 (fixed above to 12), and omits
`filler-fg` entirely (added later, when the filler hatch was screened back to its own base01
foreground). The table in this revision supersedes it for the diff keys; `cursor-bg`,
`selection-bg`, `cursor-unfocused-bg`, `pane-header-focused-fg`, and `filler-fg` are unchanged and
Expand Down
Loading