Delete F6's dead two-pane diff-view UI code (keep the baseline data layer) #34

Closed
opened 2026-06-12 08:09:15 +00:00 by benstull · 0 comments
Owner

Delete F6's dead two-pane diff-view UI code (kept only as a hidden command)

Summary

Remove the now-dead F6 two-pane diff view code (the vscode.diff toggle UI),
keeping the F6 baseline store that F7/F10 depend on as the data layer.

Problem / pain

F10 (#29) hid the F6 two-pane diff as a user surface by setting its command +
ctrl+alt+d keybinding to when:false — a deliberately small, reversible change.
The view's code (DiffViewController's toggle/pinCommand UI paths and the
vscode.diff plumbing) is still present but unreachable. Carrying dead UI code is
maintenance drag: it reads as live, future refactors must keep it compiling, and it
obscures that the preview is now the single review surface.

Who feels it

The developer maintaining the plugin — dead code costs reading/refactor effort
and invites confusion about which review surfaces are real.

Desired outcome / value

The codebase reflects reality: F6 exists only as the baseline data layer
(onDidChangeBaseline, the baseline store, the auto-advance on machine landing —
all consumed by F7/F10), and the two-pane view code is gone, along with its
hidden command/keybinding declarations in package.json.

Acceptance — "done right"

  • The F6 two-pane diff view code is removed (the toggleDiffView UI path and
    its vscode.diff plumbing).
  • The F6 baseline data layer is fully intact: DiffViewController still
    exposes the baseline + onDidChangeBaseline, still auto-advances at machine
    landings (INV-18), and F7/F10 still read it unchanged.
  • The cowriting.toggleDiffView + cowriting.pinDiffBaseline command and
    ctrl+alt+d keybinding declarations are removed from package.json (they were
    when:false).
  • npm test, npm run test:e2e, and npm run build stay green; no behavior
    change for F7/F10 (baseline/diff still drive the preview).

Scope / non-goals

In scope: deleting the dead two-pane view UI + its package.json
declarations; keeping the baseline store.

Non-goals: any change to F7/F10 rendering or to the baseline semantics
(INV-18); removing DiffViewController itself (it owns the baseline).

Assumptions · constraints · dependencies

  • Cleanup follow-up to F10 (#29). The split between "F6 view (dead)" and "F6
    baseline (live data layer)" is explicit in the F10 spec — the baseline must
    survive; only the view goes.
  • Dependency: must not touch the onDidChangeBaseline event or the auto-advance
    wiring that F7/F10 subscribe to.

Source / signal

Self-originated — capture session vscode-cowriting-plugin-0031 (2026-06-12),
from the F10 spec's deferred cleanup. Spec:
specs/coauthoring-interactive-review.md §6.7 ("Delete the view code now" — chosen
against for F10, deferred to a later cleanup PR) / §9. Follows F10 #29, F6
#17/#19.

Priority (WSJF)

Provisional: Value low (maintenance hygiene; no user-facing change) ·
Time-criticality low · Opportunity low ÷ Size small
provisional priority/P3. (Decision aid, not auto-scored.)

Decomposition

Task (refactor / dead-code removal, no independent business value). Checklist:

  • Remove the two-pane view UI from DiffViewController (toggle/pin command
    handlers + vscode.diff plumbing), keep the baseline store + onDidChangeBaseline.
  • Remove the toggleDiffView/pinDiffBaseline command + ctrl+alt+d
    keybinding declarations from package.json.
  • Confirm F7/F10 still read the baseline; full unit + E2E + build green.

Solution notes (optional — NOT a design)

Non-binding: keep DiffViewController as the baseline owner; excise only the
view-toggle paths. The E2E suite already asserts toggleDiffView is hidden — update
those assertions to "absent" once the command is removed.

<!-- Set the type/* label: type/task --> # Delete F6's dead two-pane diff-view UI code (kept only as a hidden command) ## Summary Remove the now-dead F6 two-pane diff **view** code (the `vscode.diff` toggle UI), keeping the F6 **baseline store** that F7/F10 depend on as the data layer. ## Problem / pain F10 (#29) hid the F6 two-pane diff as a user surface by setting its command + `ctrl+alt+d` keybinding to `when:false` — a deliberately small, reversible change. The view's code (`DiffViewController`'s `toggle`/`pinCommand` UI paths and the `vscode.diff` plumbing) is still present but unreachable. Carrying dead UI code is maintenance drag: it reads as live, future refactors must keep it compiling, and it obscures that the preview is now the single review surface. ## Who feels it The **developer** maintaining the plugin — dead code costs reading/refactor effort and invites confusion about which review surfaces are real. ## Desired outcome / value The codebase reflects reality: F6 exists only as the **baseline data layer** (`onDidChangeBaseline`, the baseline store, the auto-advance on machine landing — all consumed by F7/F10), and the two-pane *view* code is gone, along with its hidden command/keybinding declarations in `package.json`. ## Acceptance — "done right" - The F6 two-pane diff **view** code is removed (the `toggleDiffView` UI path and its `vscode.diff` plumbing). - The F6 **baseline** data layer is **fully intact**: `DiffViewController` still exposes the baseline + `onDidChangeBaseline`, still auto-advances at machine landings (INV-18), and F7/F10 still read it unchanged. - The `cowriting.toggleDiffView` + `cowriting.pinDiffBaseline` command and `ctrl+alt+d` keybinding declarations are removed from `package.json` (they were `when:false`). - `npm test`, `npm run test:e2e`, and `npm run build` stay green; no behavior change for F7/F10 (baseline/diff still drive the preview). ## Scope / non-goals **In scope:** deleting the dead two-pane *view* UI + its `package.json` declarations; keeping the baseline store. **Non-goals:** any change to F7/F10 rendering or to the baseline semantics (INV-18); removing `DiffViewController` itself (it owns the baseline). ## Assumptions · constraints · dependencies - Cleanup follow-up to F10 (#29). The split between "F6 view (dead)" and "F6 baseline (live data layer)" is explicit in the F10 spec — the baseline must survive; only the view goes. - Dependency: must not touch the `onDidChangeBaseline` event or the auto-advance wiring that F7/F10 subscribe to. ## Source / signal Self-originated — **capture session vscode-cowriting-plugin-0031** (2026-06-12), from the F10 spec's deferred cleanup. Spec: `specs/coauthoring-interactive-review.md` §6.7 ("Delete the view code now" — chosen against for F10, deferred to a later cleanup PR) / §9. Follows F10 `#29`, F6 `#17`/`#19`. ## Priority (WSJF) Provisional: Value **low** (maintenance hygiene; no user-facing change) · Time-criticality **low** · Opportunity **low** ÷ Size **small** → **provisional priority/P3**. (Decision aid, not auto-scored.) ## Decomposition Task (refactor / dead-code removal, no independent business value). Checklist: - [ ] Remove the two-pane view UI from `DiffViewController` (toggle/pin command handlers + `vscode.diff` plumbing), keep the baseline store + `onDidChangeBaseline`. - [ ] Remove the `toggleDiffView`/`pinDiffBaseline` command + `ctrl+alt+d` keybinding declarations from `package.json`. - [ ] Confirm F7/F10 still read the baseline; full unit + E2E + build green. ## Solution notes (optional — NOT a design) Non-binding: keep `DiffViewController` as the baseline owner; excise only the view-toggle paths. The E2E suite already asserts `toggleDiffView` is hidden — update those assertions to "absent" once the command is removed.
benstull added the type/taskpriority/P3 labels 2026-06-12 08:09:15 +00:00
Sign in to join this conversation.