diff --git a/.claude-plugin/skills/revdiff/SKILL.md b/.claude-plugin/skills/revdiff/SKILL.md index 65333bbd..55d3ecc7 100644 --- a/.claude-plugin/skills/revdiff/SKILL.md +++ b/.claude-plugin/skills/revdiff/SKILL.md @@ -122,6 +122,8 @@ When you are launching revdiff for the user (e.g., right after a refactor or ana **When the recent change likely created new untracked files** (new packages, new test files, new docs, new scripts that haven't been `git add`-ed yet), pass `--untracked` so those files appear in the tree. Use this in working-tree mode (no ref, no `--staged`); skip it for ref-to-ref reviews where untracked files are not part of the historical diff. +**When the user explicitly requests the file tree on a side**, pass `--tree-position=left` or `--tree-position=right`. Otherwise omit it and respect the user's config. + Run the launcher through the override-chain resolver: ```bash diff --git a/.claude-plugin/skills/revdiff/references/config.md b/.claude-plugin/skills/revdiff/references/config.md index 0f5e825d..87e7240b 100644 --- a/.claude-plugin/skills/revdiff/references/config.md +++ b/.claude-plugin/skills/revdiff/references/config.md @@ -21,6 +21,7 @@ Then uncomment and edit the values you want to change. | `--staged` | `REVDIFF_STAGED` | Show staged changes | `false` | | `--untracked` | `REVDIFF_UNTRACKED` | Show untracked files in the tree | `false` | | `--tree-width` | `REVDIFF_TREE_WIDTH` | File tree panel width in units (1-10) | `2` | +| `--tree-position` | `REVDIFF_TREE_POSITION` | File tree and markdown TOC position (`left` or `right`) | `left` | | `--tab-width` | `REVDIFF_TAB_WIDTH` | Spaces per tab character | `4` | | `--no-colors` | `REVDIFF_NO_COLORS` | Disable all colors including syntax highlighting | `false` | | `--no-status-bar` | `REVDIFF_NO_STATUS_BAR` | Hide the status bar | `false` | @@ -152,7 +153,7 @@ unmap q map ctrl+d half_page_down ``` -Available actions: `down`, `up`, `page_down`, `page_up`, `half_page_down`, `half_page_up`, `home`, `end`, `scroll_left`, `scroll_right`, `scroll_center`, `scroll_top`, `scroll_bottom`, `scroll_diff_down`, `scroll_diff_up`, `next_item`, `prev_item`, `jump_file`, `next_hunk`, `prev_hunk`, `open_file_in_editor`, `toggle_pane`, `focus_tree`, `focus_diff`, `search`, `confirm`, `annotate_file`, `delete_annotation`, `annot_list`, `open_editor`, `next_annotation`, `prev_annotation`, `flush_output`, `toggle_collapsed`, `toggle_compact`, `toggle_wrap`, `toggle_tree`, `toggle_line_numbers`, `toggle_blame`, `toggle_word_diff`, `toggle_hunk`, `toggle_untracked`, `mark_reviewed`, `theme_select`, `filter`, `info`, `reload`, `quit`, `discard_quit`, `help`, `dismiss` +Available actions: `down`, `up`, `page_down`, `page_up`, `half_page_down`, `half_page_up`, `home`, `end`, `scroll_left`, `scroll_right`, `scroll_center`, `scroll_top`, `scroll_bottom`, `scroll_diff_down`, `scroll_diff_up`, `next_item`, `prev_item`, `jump_file`, `next_hunk`, `prev_hunk`, `open_file_in_editor`, `toggle_pane`, `focus_left`, `focus_right`, `focus_tree`, `focus_diff`, `search`, `confirm`, `annotate_file`, `delete_annotation`, `annot_list`, `open_editor`, `next_annotation`, `prev_annotation`, `flush_output`, `toggle_collapsed`, `toggle_compact`, `toggle_wrap`, `toggle_tree`, `toggle_line_numbers`, `toggle_blame`, `toggle_word_diff`, `toggle_hunk`, `toggle_untracked`, `mark_reviewed`, `theme_select`, `filter`, `info`, `reload`, `quit`, `discard_quit`, `help`, `dismiss` Fixed modal keys (Enter, Esc in annotation/search input, confirm discard) are not remappable. Keymap-resolved actions like `open_editor` work during annotation input and can be rebound. Chord bindings do not fire during text input — use single-key `ctrl+*` bindings for actions that need to work during annotation input. diff --git a/.claude-plugin/skills/revdiff/references/usage.md b/.claude-plugin/skills/revdiff/references/usage.md index c3bf536a..495e8968 100644 --- a/.claude-plugin/skills/revdiff/references/usage.md +++ b/.claude-plugin/skills/revdiff/references/usage.md @@ -35,7 +35,7 @@ When a diff contains exactly one file, revdiff automatically hides the file tree ## Markdown TOC Navigation -When reviewing a single markdown file in context-only mode (e.g., `revdiff --only=README.md`), a table-of-contents pane appears on the left listing all markdown headers with indentation by level. Use `Tab` to switch between TOC and diff, `j`/`k` to navigate headers, `n`/`p` to jump to next/prev header from either pane, `Enter` to jump to a header. The TOC highlights the current section as you scroll. Headers inside fenced code blocks are excluded. +When reviewing a single markdown file in context-only mode (e.g., `revdiff --only=README.md`), a table-of-contents pane appears on the configured tree side (left by default) listing all markdown headers with indentation by level. Use `Tab` to switch between TOC and diff, `j`/`k` to navigate headers, `n`/`p` to jump to next/prev header from either pane, `Enter` to jump to a header. The TOC highlights the current section as you scroll. Headers inside fenced code blocks are excluded. ## All-Files Mode diff --git a/README.md b/README.md index 9d0d79be..7f01a23d 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Built for a specific use case: reviewing code changes, plans, and documents with - Blame gutter: shows author name and commit age per line, toggle with `B` - Annotate any line in the diff (added, removed, or context) plus file-level notes - Single-file auto-detection: when a diff contains exactly one file, hides the tree pane and gives full terminal width to the diff view -- Two-pane TUI: file tree (left) + colorized diff viewport (right) +- Two-pane TUI: file tree + colorized diff viewport - Vim-style `/` search within diff with `n`/`N` match navigation - Hunk navigation to jump between change groups - Annotation list popup (`@`): browse all annotations across files, jump to any annotation @@ -354,6 +354,7 @@ Positional arguments support several forms: | `--staged` | Show staged changes, env: `REVDIFF_STAGED` | `false` | | `--untracked` | Show untracked files in the tree, env: `REVDIFF_UNTRACKED` | `false` | | `--tree-width` | File tree panel width in units (1-10), env: `REVDIFF_TREE_WIDTH` | `2` | +| `--tree-position` | File tree and markdown TOC position (`left` or `right`), env: `REVDIFF_TREE_POSITION` | `left` | | `--tab-width` | Number of spaces per tab character, env: `REVDIFF_TAB_WIDTH` | `4` | | `--no-colors` | Disable all colors including syntax highlighting, env: `REVDIFF_NO_COLORS` | `false` | | `--no-status-bar` | Hide the status bar, env: `REVDIFF_NO_STATUS_BAR` | `false` | @@ -665,7 +666,7 @@ revdiff HEAD~3 --description-file=.review-description.md ### Markdown TOC Navigation -When reviewing a single markdown file in context-only mode (e.g., `revdiff --only=README.md` or `printf '# title\n' | revdiff --stdin --stdin-name plan.md`), revdiff shows a table-of-contents pane on the left listing all markdown headers. Use `Tab` to switch focus between the TOC and diff panes, `j`/`k` to navigate headers, and `Enter` to jump to a header in the diff. The TOC automatically highlights the current section as you scroll through the file. +When reviewing a single markdown file in context-only mode (e.g., `revdiff --only=README.md` or `printf '# title\n' | revdiff --stdin --stdin-name plan.md`), revdiff shows a table-of-contents pane on the configured side listing all markdown headers. Use `Tab` to switch focus between the TOC and diff panes, `j`/`k` to navigate headers, and `Enter` to jump to a header in the diff. The TOC automatically highlights the current section as you scroll through the file. This mode activates when all three conditions are met: single file, markdown extension (`.md`/`.markdown`), and all lines are context (no diff changes). Headers inside fenced code blocks are excluded from the TOC. @@ -885,7 +886,7 @@ When the leader is pressed, the status bar shows `Pending: ctrl+w, esc to cancel **File/Hunk:** `next_item`, `prev_item`, `jump_file`, `next_hunk`, `prev_hunk`, `open_file_in_editor` -**Pane:** `toggle_pane`, `focus_tree`, `focus_diff` +**Pane:** `toggle_pane`, `focus_left`, `focus_right`, `focus_tree`, `focus_diff` **Search:** `search` diff --git a/app/config.go b/app/config.go index a7327185..d481e004 100644 --- a/app/config.go +++ b/app/config.go @@ -9,6 +9,8 @@ import ( "strings" "github.com/jessevdk/go-flags" + + "github.com/umputun/revdiff/app/ui" ) type options struct { @@ -20,6 +22,7 @@ type options struct { Staged bool `long:"staged" ini-name:"staged" env:"REVDIFF_STAGED" description:"show staged changes"` Untracked bool `long:"untracked" ini-name:"untracked" env:"REVDIFF_UNTRACKED" description:"show untracked files in the tree"` TreeWidth int `long:"tree-width" ini-name:"tree-width" env:"REVDIFF_TREE_WIDTH" default:"2" description:"file tree panel width in units (1-10, default 2 of 10)"` + TreePosition string `long:"tree-position" ini-name:"tree-position" env:"REVDIFF_TREE_POSITION" choice:"left" choice:"right" default:"left" description:"file tree and markdown TOC position"` TabWidth int `long:"tab-width" ini-name:"tab-width" env:"REVDIFF_TAB_WIDTH" default:"4" description:"number of spaces per tab character"` NoColors bool `long:"no-colors" ini-name:"no-colors" env:"REVDIFF_NO_COLORS" description:"disable all colors including syntax highlighting"` NoStatusBar bool `long:"no-status-bar" ini-name:"no-status-bar" env:"REVDIFF_NO_STATUS_BAR" description:"hide the status bar"` @@ -119,6 +122,14 @@ func (o options) startupUntracked() bool { return true } +// treePosition resolves the --tree-position flag value to its typed position. +func (o options) treePosition() ui.TreePosition { + if o.TreePosition == "right" { + return ui.TreePositionRight + } + return ui.TreePositionLeft +} + // parseArgs parses CLI arguments with config file support. // config file is loaded first, then CLI args override. // precedence: CLI flags > env vars > config file > built-in defaults. diff --git a/app/config_test.go b/app/config_test.go index a3ec94db..bb1f0e9e 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -21,6 +21,7 @@ func TestParseArgs_Defaults(t *testing.T) { opts, err := parseArgs(noConfigArgs(t)) require.NoError(t, err) assert.Equal(t, 2, opts.TreeWidth) + assert.Equal(t, "left", opts.TreePosition) assert.Equal(t, 4, opts.TabWidth) assert.Equal(t, "catppuccin-macchiato", opts.ChromaStyle) assert.Equal(t, "💬", opts.AnnotationMarker) @@ -48,6 +49,36 @@ func TestParseArgs_Defaults(t *testing.T) { assert.Equal(t, "catppuccin-latte", opts.AutoThemeLight) } +func TestParseArgs_TreePosition(t *testing.T) { + t.Run("flag", func(t *testing.T) { + opts, err := parseArgs(append(noConfigArgs(t), "--tree-position=right")) + require.NoError(t, err) + assert.Equal(t, "right", opts.TreePosition) + }) + + t.Run("env", func(t *testing.T) { + t.Setenv("REVDIFF_TREE_POSITION", "right") + opts, err := parseArgs(noConfigArgs(t)) + require.NoError(t, err) + assert.Equal(t, "right", opts.TreePosition) + }) + + t.Run("config file", func(t *testing.T) { + cfgPath := filepath.Join(t.TempDir(), "config") + err := os.WriteFile(cfgPath, []byte("[Application Options]\ntree-position = right\n"), 0o600) + require.NoError(t, err) + + opts, err := parseArgs([]string{"--config", cfgPath}) + require.NoError(t, err) + assert.Equal(t, "right", opts.TreePosition) + }) + + t.Run("invalid", func(t *testing.T) { + _, err := parseArgs(append(noConfigArgs(t), "--tree-position=center")) + require.Error(t, err) + }) +} + func TestParseArgs_NoConfirmDiscard(t *testing.T) { t.Run("flag", func(t *testing.T) { opts, err := parseArgs(append(noConfigArgs(t), "--no-confirm-discard")) @@ -855,6 +886,7 @@ func TestDumpConfig(t *testing.T) { assert.Contains(t, output, "[Application Options]") assert.Contains(t, output, "chroma-style = catppuccin-macchiato") + assert.Contains(t, output, "tree-position = left") assert.Contains(t, output, "cross-file-hunks = false") assert.Contains(t, output, "exit-code-on-annotations = false") assert.Contains(t, output, "no-mouse = false") diff --git a/app/keymap/keymap.go b/app/keymap/keymap.go index c3baa85f..35085af3 100644 --- a/app/keymap/keymap.go +++ b/app/keymap/keymap.go @@ -40,6 +40,8 @@ const ( ActionNextHunk Action = "next_hunk" ActionPrevHunk Action = "prev_hunk" ActionTogglePane Action = "toggle_pane" + ActionFocusLeft Action = "focus_left" + ActionFocusRight Action = "focus_right" ActionFocusTree Action = "focus_tree" ActionFocusDiff Action = "focus_diff" ActionSearch Action = "search" @@ -85,7 +87,8 @@ var validActions = map[Action]bool{ ActionScrollDiffDown: true, ActionScrollDiffUp: true, ActionNextItem: true, ActionPrevItem: true, ActionJumpFile: true, ActionNextHunk: true, ActionPrevHunk: true, - ActionTogglePane: true, ActionFocusTree: true, ActionFocusDiff: true, + ActionTogglePane: true, ActionFocusLeft: true, ActionFocusRight: true, + ActionFocusTree: true, ActionFocusDiff: true, ActionSearch: true, ActionConfirm: true, ActionAnnotateFile: true, ActionDeleteAnnotation: true, ActionAnnotList: true, ActionNextAnnotation: true, ActionPrevAnnotation: true, @@ -212,6 +215,8 @@ func defaultDescriptions() []HelpEntry { // pane {ActionTogglePane, "toggle pane focus", SectionPane}, + {ActionFocusLeft, "focus left pane", SectionPane}, + {ActionFocusRight, "focus right pane", SectionPane}, {ActionFocusTree, "focus tree pane", SectionPane}, {ActionFocusDiff, "focus diff pane", SectionPane}, @@ -278,8 +283,8 @@ func defaultBindings() map[string]Action { "[": ActionPrevHunk, "e": ActionOpenFileInEditor, "tab": ActionTogglePane, - "h": ActionFocusTree, - "l": ActionFocusDiff, + "h": ActionFocusLeft, + "l": ActionFocusRight, "/": ActionSearch, "a": ActionConfirm, "enter": ActionConfirm, diff --git a/app/keymap/keymap_test.go b/app/keymap/keymap_test.go index 31b7f880..24afab72 100644 --- a/app/keymap/keymap_test.go +++ b/app/keymap/keymap_test.go @@ -33,7 +33,7 @@ func TestDefault_allExpectedBindings(t *testing.T) { {"n", ActionNextItem}, {"N", ActionPrevItem}, {"p", ActionPrevItem}, {"ctrl+p", ActionJumpFile}, {"]", ActionNextHunk}, {"[", ActionPrevHunk}, {"e", ActionOpenFileInEditor}, - {"tab", ActionTogglePane}, {"h", ActionFocusTree}, {"l", ActionFocusDiff}, + {"tab", ActionTogglePane}, {"h", ActionFocusLeft}, {"l", ActionFocusRight}, {"/", ActionSearch}, {"a", ActionConfirm}, {"enter", ActionConfirm}, {"A", ActionAnnotateFile}, {"d", ActionDeleteAnnotation}, {"@", ActionAnnotList}, {"ctrl+e", ActionOpenEditor}, @@ -404,6 +404,8 @@ func TestActionScrollConstants_NoDefaultBindings(t *testing.T) { func TestIsValidAction(t *testing.T) { assert.True(t, IsValidAction(ActionQuit)) assert.True(t, IsValidAction(ActionDown)) + assert.True(t, IsValidAction(ActionFocusLeft)) + assert.True(t, IsValidAction(ActionFocusRight)) assert.True(t, IsValidAction(ActionInfo)) assert.True(t, IsValidAction(Action("commit_info")), "deprecated alias must validate") assert.False(t, IsValidAction(Action("nonexistent"))) diff --git a/app/main.go b/app/main.go index be91347d..f03918a6 100644 --- a/app/main.go +++ b/app/main.go @@ -231,6 +231,7 @@ func run(opts options) (int, error) { Ref: opts.ref(), Staged: opts.Staged, TreeWidthRatio: opts.TreeWidth, + TreePosition: opts.treePosition(), Only: opts.Only, WorkDir: workDir, SourceEditor: sourceEditorPolicy(opts, workDir), diff --git a/app/ui/doc.go b/app/ui/doc.go index 4e66f160..f7870845 100644 --- a/app/ui/doc.go +++ b/app/ui/doc.go @@ -59,7 +59,7 @@ // (styleResolver, styleRenderer, sgrProcessor) defined in model.go; concrete // implementations live in the style sub-package. // -// Left-pane navigation components live in the [sidepane] sub-package (app/ui/sidepane/). +// Navigation-pane components live in the [sidepane] sub-package (app/ui/sidepane/). // It owns the file tree (FileTree) and markdown table-of-contents (TOC) types, // including cursor/offset management, entry parsing, and rendering logic. // Model holds sidepane types through consumer-side interfaces (FileTreeComponent, diff --git a/app/ui/model.go b/app/ui/model.go index 64f1972a..9a5c751b 100644 --- a/app/ui/model.go +++ b/app/ui/model.go @@ -278,6 +278,14 @@ type TOCComponent interface { Render(r sidepane.TOCRender) string } +// TreePosition is the side of the screen where the file tree or markdown TOC renders. +type TreePosition int + +const ( + TreePositionLeft TreePosition = iota + TreePositionRight +) + // pane identifies which pane has focus. type pane int @@ -332,6 +340,8 @@ type modelConfigState struct { annotPrefix string // cached: marker + " " annotFilePrefix string // cached: marker + " file: " outputPath string // --output destination for the O in-session flush; empty disables it + + treePosition TreePosition // side the file tree or markdown TOC renders on } // layoutState holds viewport and layout concerns that change on resize and pane toggles. @@ -788,6 +798,8 @@ type ModelConfig struct { // disables the flush (there is no file to write to); a non-empty path enables // it. Copied into modelConfigState.outputPath as a plain value. OutputPath string + + TreePosition TreePosition // side the file tree or markdown TOC renders on } // NewModel creates a new Model from the given configuration. All dependencies @@ -887,6 +899,7 @@ func NewModel(cfg ModelConfig) (Model, error) { noConfirmReload: cfg.NoConfirmReload, crossFileHunks: cfg.CrossFileHunks, treeWidthRatio: cfg.TreeWidthRatio, + treePosition: cfg.TreePosition, tabSpaces: strings.Repeat(" ", cfg.TabWidth), wrapIndent: max(0, cfg.WrapIndent), annotPrefix: cfg.AnnotationMarker + " ", @@ -1057,11 +1070,29 @@ func (m Model) handleKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m.dispatchAction(action) } +// resolveDirectionalFocus maps spatial focus actions to semantic pane actions. +// Explicit user bindings to focus_tree or focus_diff bypass this mapping. +func (m Model) resolveDirectionalFocus(action keymap.Action) keymap.Action { + left, right := keymap.ActionFocusTree, keymap.ActionFocusDiff + if m.cfg.treePosition == TreePositionRight { + left, right = right, left + } + switch action { + case keymap.ActionFocusLeft: + return left + case keymap.ActionFocusRight: + return right + default: + return action + } +} + // dispatchAction routes a resolved keymap action through overlay-open, the // global action switch, and the pane-specific nav fallback. It is the unified // dispatch path shared by keymap-resolved single keys (handleKey) and by // chord-resolved actions (handleChordSecond). func (m Model) dispatchAction(action keymap.Action) (tea.Model, tea.Cmd) { + action = m.resolveDirectionalFocus(action) if model, cmd, ok := m.handleOverlayOpen(action); ok { return model, cmd } diff --git a/app/ui/model_test.go b/app/ui/model_test.go index 5219699f..221335b8 100644 --- a/app/ui/model_test.go +++ b/app/ui/model_test.go @@ -501,19 +501,43 @@ func TestModel_TreeNavigation(t *testing.T) { } func TestModel_FocusSwitching(t *testing.T) { + for _, tc := range []struct { + name string + treePos TreePosition + toDiff rune // key that moves focus tree→diff + toTree rune // key that moves focus diff→tree + }{ + {name: "left tree", toDiff: 'l', toTree: 'h'}, + {name: "right tree", treePos: TreePositionRight, toDiff: 'h', toTree: 'l'}, + } { + t.Run(tc.name, func(t *testing.T) { + m := testModel([]string{"a.go"}, nil) + m.tree = testNewFileTree([]string{"a.go"}) + m.cfg.treePosition = tc.treePos + m.file.name = "a.go" // pretend a file is loaded + m.layout.focus = paneTree + + result, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{tc.toDiff}}) + model := result.(Model) + assert.Equal(t, paneDiff, model.layout.focus) + + result, _ = model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{tc.toTree}}) + model = result.(Model) + assert.Equal(t, paneTree, model.layout.focus) + }) + } +} + +func TestModel_FocusSwitching_UserSemanticBinding(t *testing.T) { m := testModel([]string{"a.go"}, nil) m.tree = testNewFileTree([]string{"a.go"}) - m.file.name = "a.go" // pretend a file is loaded - m.layout.focus = paneTree + m.cfg.treePosition = TreePositionRight + m.keymap.Bind("h", keymap.ActionFocusTree) + m.file.name = "a.go" + m.layout.focus = paneDiff - // l switches to diff pane - result, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) + result, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'h'}}) model := result.(Model) - assert.Equal(t, paneDiff, model.layout.focus) - - // h switches back to tree - result, _ = model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'h'}}) - model = result.(Model) assert.Equal(t, paneTree, model.layout.focus) } diff --git a/app/ui/mouse.go b/app/ui/mouse.go index d686e7fa..d5988c94 100644 --- a/app/ui/mouse.go +++ b/app/ui/mouse.go @@ -90,11 +90,21 @@ func (m Model) treeTopRow() int { return 1 } +// treePaneXRange returns the half-open screen column range [start, end) of the +// tree pane block: left border + treeWidth content columns + right border. +// the block hugs the right edge when the tree renders on the right. +func (m Model) treePaneXRange() (start, end int) { + if m.cfg.treePosition == TreePositionRight { + start = m.layout.width - m.layout.treeWidth - 2 + } + return start, start + m.layout.treeWidth + 2 +} + // hitTest classifies a screen coordinate into a hitZone for mouse-event routing. // the classification is pure arithmetic over m.layout state and does not // inspect any dynamic UI content. ordering matters: status bar is checked -// first (y at bottom), then x is used to split tree vs diff columns, and -// finally y is used within each column to reject the diff header row or tree +// first (y at bottom), then x is used to identify the configured tree side, +// and finally y is used within each pane to reject the diff header row or tree // top border. func (m Model) hitTest(x, y int) hitZone { if x < 0 || y < 0 || x >= m.layout.width || y >= m.layout.height { @@ -111,10 +121,7 @@ func (m Model) hitTest(x, y int) hitZone { return hitNone } - // tree block spans columns [0, treeWidth+1] when visible: left border + - // treeWidth content columns + right border = treeWidth+2 columns total. - // diff block picks up at column treeWidth+2. - if !m.treePaneHidden() && x < m.layout.treeWidth+2 { + if start, end := m.treePaneXRange(); !m.treePaneHidden() && x >= start && x < end { if y < m.treeTopRow() { return hitNone } diff --git a/app/ui/mouse_test.go b/app/ui/mouse_test.go index 7d3c7e83..0704e027 100644 --- a/app/ui/mouse_test.go +++ b/app/ui/mouse_test.go @@ -93,6 +93,34 @@ func TestModel_hitTest(t *testing.T) { {name: "status bar at x=0", setup: func(m *Model) {}, x: 0, y: 39, want: hitStatus}, {name: "tree-diff boundary: last tree column", setup: func(m *Model) {}, x: 37, y: 10, want: hitTree}, {name: "tree-diff boundary: first diff column", setup: func(m *Model) {}, x: 38, y: 10, want: hitDiff}, + { + name: "right tree: last diff column", + setup: func(m *Model) { + m.cfg.treePosition = TreePositionRight + }, + x: 81, y: 10, want: hitDiff, + }, + { + name: "right tree: first tree column", + setup: func(m *Model) { + m.cfg.treePosition = TreePositionRight + }, + x: 82, y: 10, want: hitTree, + }, + { + name: "right tree: tree top border", + setup: func(m *Model) { + m.cfg.treePosition = TreePositionRight + }, + x: 100, y: 0, want: hitNone, + }, + { + name: "right tree: diff header on left", + setup: func(m *Model) { + m.cfg.treePosition = TreePositionRight + }, + x: 5, y: 1, want: hitHeader, + }, {name: "x negative", setup: func(m *Model) {}, x: -1, y: 10, want: hitNone}, {name: "y negative", setup: func(m *Model) {}, x: 60, y: -1, want: hitNone}, {name: "x out of bounds (= width)", setup: func(m *Model) {}, x: 120, y: 10, want: hitNone}, @@ -432,6 +460,16 @@ func TestModel_HandleMouse_WheelInTreeMovesTreeCursor(t *testing.T) { assert.Equal(t, "ab.go", model.tree.SelectedFile(), "single wheel-down notch must advance tree cursor by exactly one entry") }) + t.Run("right-positioned tree receives wheel events on the right", func(t *testing.T) { + m := mouseTestModel(t, files, diffs) + m.cfg.treePosition = TreePositionRight + require.Equal(t, "aa.go", m.tree.SelectedFile()) + + result, _ := m.Update(wheelMsg(tea.MouseButtonWheelDown, 100, 3, false)) + model := result.(Model) + assert.Equal(t, "ab.go", model.tree.SelectedFile()) + }) + t.Run("plain wheel-up retreats exactly one entry", func(t *testing.T) { m := mouseTestModel(t, files, diffs) m.tree.SelectByPath("ac.go") // cursor at entry 2 diff --git a/app/ui/view.go b/app/ui/view.go index 80c9e70c..d150c5ac 100644 --- a/app/ui/view.go +++ b/app/ui/view.go @@ -62,7 +62,7 @@ func (m Model) View() string { mainView = m.applyScrollbar(diffPane) case m.file.singleFile && m.file.mdTOC != nil: - // single-file markdown with TOC: two-pane layout with TOC in left pane + // single-file markdown with TOC: use the configured navigation pane side tocContent := m.file.mdTOC.Render(sidepane.TOCRender{Width: m.layout.treeWidth, Height: ph, Focused: m.layout.focus == paneTree, Resolver: m.resolver}) mainView = m.renderTwoPaneLayout(tocContent, diffContent, m.file.mdTOC.ScrollState(), ph, diffPaneW) @@ -82,12 +82,12 @@ func (m Model) View() string { return lipgloss.JoinVertical(lipgloss.Left, mainView, status) } -// renderTwoPaneLayout renders a two-pane layout with left (tree/TOC) and right (diff) content. +// renderTwoPaneLayout renders the navigation and diff panes in the configured order. // applies focus-based pane styles, background padding, scrollbars, and joins horizontally. // diffPaneW is the inner width caller passed to truncateHeaderTitle and must // match the lipgloss Width() applied here — single source of truth for the // scrollbar's single-line-header invariant. -func (m Model) renderTwoPaneLayout(leftContent, diffContent string, leftScroll sidepane.ScrollState, ph, diffPaneW int) string { +func (m Model) renderTwoPaneLayout(navigationContent, diffContent string, navigationScroll sidepane.ScrollState, ph, diffPaneW int) string { treeStyle := m.resolver.Style(style.StyleKeyTreePane) diffStyle := m.resolver.Style(style.StyleKeyDiffPane) if m.layout.focus == paneTree { @@ -96,14 +96,14 @@ func (m Model) renderTwoPaneLayout(leftContent, diffContent string, leftScroll s diffStyle = m.resolver.Style(style.StyleKeyDiffPaneActive) } - leftContent = m.padContentBg(leftContent, m.layout.treeWidth, m.resolver.Color(style.ColorKeyTreePaneBg)) + navigationContent = m.padContentBg(navigationContent, m.layout.treeWidth, m.resolver.Color(style.ColorKeyTreePaneBg)) diffContent = m.padContentBg(diffContent, diffPaneW, m.resolver.Color(style.ColorKeyDiffPaneBg)) - leftPane := treeStyle. + navigationPane := treeStyle. Width(m.layout.treeWidth). Height(ph). - Render(leftContent) - leftPane = m.applyNavigationScrollbar(leftPane, leftScroll) + Render(navigationContent) + navigationPane = m.applyNavigationScrollbar(navigationPane, navigationScroll) diffPane := diffStyle. Width(diffPaneW). @@ -111,7 +111,10 @@ func (m Model) renderTwoPaneLayout(leftContent, diffContent string, leftScroll s Render(diffContent) diffPane = m.applyScrollbar(diffPane) - return lipgloss.JoinHorizontal(lipgloss.Top, leftPane, diffPane) + if m.cfg.treePosition == TreePositionRight { + return lipgloss.JoinHorizontal(lipgloss.Top, diffPane, navigationPane) + } + return lipgloss.JoinHorizontal(lipgloss.Top, navigationPane, diffPane) } // truncateHeaderTitle returns the diff pane header text shortened to fit diff --git a/app/ui/view_test.go b/app/ui/view_test.go index 245e9b3f..bb28fe0f 100644 --- a/app/ui/view_test.go +++ b/app/ui/view_test.go @@ -607,6 +607,63 @@ func TestModel_ViewSingleFileMode(t *testing.T) { }) } +func TestModel_ViewTreePosition(t *testing.T) { + // assertLeftOf makes each call site state the expected pane order explicitly + // instead of mirroring the production treePosition branch + assertLeftOf := func(t *testing.T, view, leftLabel, rightLabel string) { + t.Helper() + lines := strings.Split(ansi.Strip(view), "\n") + labelColumn := func(label string) int { + for _, line := range lines { + if idx := strings.Index(line, label); idx >= 0 { + return idx + } + } + return -1 + } + + leftIdx, rightIdx := labelColumn(leftLabel), labelColumn(rightLabel) + require.NotEqual(t, -1, leftIdx, "label %q must appear in the rendered view", leftLabel) + require.NotEqual(t, -1, rightIdx, "label %q must appear in the rendered view", rightLabel) + assert.Less(t, leftIdx, rightIdx) + } + + for _, tc := range []struct { + name string + treePos TreePosition + leftLabel, rightLabel string + }{ + {name: "file tree on left", leftLabel: "tree.go", rightLabel: "current.go"}, + {name: "file tree on right", treePos: TreePositionRight, leftLabel: "current.go", rightLabel: "tree.go"}, + } { + t.Run(tc.name, func(t *testing.T) { + m := testModel([]string{"tree.go", "other.go"}, nil) + m.tree = testNewFileTree([]string{"tree.go", "other.go"}) + m.file.name = "current.go" + m.cfg.treePosition = tc.treePos + m.cfg.noStatusBar = true + + assertLeftOf(t, m.View(), tc.leftLabel, tc.rightLabel) + }) + } + + t.Run("markdown TOC follows tree position", func(t *testing.T) { + m := testModel([]string{"plan.md"}, nil) + m.tree = testNewFileTree([]string{"plan.md"}) + m.file.singleFile = true + m.file.mdTOC = sidepane.ParseTOC( + []diff.DiffLine{{NewNum: 1, Content: "# Navigation section", ChangeType: diff.ChangeContext}}, + "plan.md", + ) + require.NotNil(t, m.file.mdTOC) + m.file.name = "plan.md" + m.cfg.treePosition = TreePositionRight + m.cfg.noStatusBar = true + + assertLeftOf(t, m.View(), "plan.md", "Navigation section") + }) +} + func TestModel_ViewRenameHeader(t *testing.T) { t.Run("renamed file header shows old to new", func(t *testing.T) { m := testModel([]string{"new.go"}, nil) @@ -1036,6 +1093,21 @@ func TestModel_HKeySwitchesToTOC(t *testing.T) { assert.Equal(t, paneTree, model.layout.focus, "h key should switch to TOC pane") }) + t.Run("l key in diff pane switches to right-positioned TOC", func(t *testing.T) { + m := testModel([]string{"README.md"}, map[string][]diff.DiffLine{"README.md": mdLines}) + m.file.singleFile = true + m.file.mdTOC = sidepane.ParseTOC(mdLines, "README.md") + require.NotNil(t, m.file.mdTOC) + m.file.name = "README.md" + m.file.lines = mdLines + m.layout.focus = paneDiff + m.cfg.treePosition = TreePositionRight + + result, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'l'}}) + model := result.(Model) + assert.Equal(t, paneTree, model.layout.focus, "l key should switch to the right-positioned TOC pane") + }) + t.Run("h key no-op in single-file without TOC", func(t *testing.T) { m := testModel([]string{"main.go"}, nil) m.file.singleFile = true diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f82cea8c..78047d3f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -146,7 +146,7 @@ Three main types: `display.go` holds two package-level functions rather than methods on those types: `SanitizeFilenameForDisplay()` strips control, ANSI/OSC, and bidi sequences out of repository-supplied filenames, and `TruncateLeftToWidth()` left-truncates with an ellipsis. Both are shared by the diff-pane header, the status bar, and the file picker; any new filename-rendering surface must route through them. -### app/ui/sidepane/ — left-pane navigation +### app/ui/sidepane/ — navigation pane Two independent component types, both with cursor/offset management, rendering, and keyboard navigation: - **`FileTree`** — file tree sidebar. Supports navigation (`Move`/`StepFile`), filtering (annotated-only), semantic-fingerprint reviewed tracking, directory grouping. File-list reloads revalidate only paths reviewed before the load; marks added during the load are reconciled when that file's refreshed diff arrives. `VisibleFiles()` exposes file paths in rendered order after active filters for consumers such as the file picker. diff --git a/plugins/codex/skills/revdiff/SKILL.md b/plugins/codex/skills/revdiff/SKILL.md index 54a717c1..b73a8699 100644 --- a/plugins/codex/skills/revdiff/SKILL.md +++ b/plugins/codex/skills/revdiff/SKILL.md @@ -138,6 +138,8 @@ When you are launching revdiff for the user (e.g., right after a refactor or ana **When the recent change likely created new untracked files** (new packages, new test files, new docs, new scripts that haven't been `git add`-ed yet), pass `--untracked` so those files appear in the tree. Use this in working-tree mode (no ref, no `--staged`); skip it for ref-to-ref reviews where untracked files are not part of the historical diff. +**When the user explicitly requests the file tree on a side**, pass `--tree-position=left` or `--tree-position=right`. Otherwise omit it and respect the user's config. + Run the launcher script: ```bash diff --git a/plugins/codex/skills/revdiff/references/config.md b/plugins/codex/skills/revdiff/references/config.md index baa541d2..48c91b24 100644 --- a/plugins/codex/skills/revdiff/references/config.md +++ b/plugins/codex/skills/revdiff/references/config.md @@ -21,6 +21,7 @@ Then uncomment and edit the values you want to change. | `--staged` | `REVDIFF_STAGED` | Show staged changes | `false` | | `--untracked` | `REVDIFF_UNTRACKED` | Show untracked files in the tree | `false` | | `--tree-width` | `REVDIFF_TREE_WIDTH` | File tree panel width in units (1-10) | `2` | +| `--tree-position` | `REVDIFF_TREE_POSITION` | File tree and markdown TOC position (`left` or `right`) | `left` | | `--tab-width` | `REVDIFF_TAB_WIDTH` | Spaces per tab character | `4` | | `--no-colors` | `REVDIFF_NO_COLORS` | Disable all colors including syntax highlighting | `false` | | `--no-status-bar` | `REVDIFF_NO_STATUS_BAR` | Hide the status bar | `false` | @@ -147,7 +148,7 @@ unmap q map ctrl+d half_page_down ``` -Available actions: `down`, `up`, `page_down`, `page_up`, `half_page_down`, `half_page_up`, `home`, `end`, `scroll_left`, `scroll_right`, `scroll_center`, `scroll_top`, `scroll_bottom`, `scroll_diff_down`, `scroll_diff_up`, `next_item`, `prev_item`, `jump_file`, `next_hunk`, `prev_hunk`, `open_file_in_editor`, `toggle_pane`, `focus_tree`, `focus_diff`, `search`, `confirm`, `annotate_file`, `delete_annotation`, `annot_list`, `open_editor`, `next_annotation`, `prev_annotation`, `flush_output`, `toggle_collapsed`, `toggle_compact`, `toggle_wrap`, `toggle_tree`, `toggle_line_numbers`, `toggle_blame`, `toggle_word_diff`, `toggle_hunk`, `toggle_untracked`, `mark_reviewed`, `theme_select`, `filter`, `info`, `reload`, `quit`, `discard_quit`, `help`, `dismiss` +Available actions: `down`, `up`, `page_down`, `page_up`, `half_page_down`, `half_page_up`, `home`, `end`, `scroll_left`, `scroll_right`, `scroll_center`, `scroll_top`, `scroll_bottom`, `scroll_diff_down`, `scroll_diff_up`, `next_item`, `prev_item`, `jump_file`, `next_hunk`, `prev_hunk`, `open_file_in_editor`, `toggle_pane`, `focus_left`, `focus_right`, `focus_tree`, `focus_diff`, `search`, `confirm`, `annotate_file`, `delete_annotation`, `annot_list`, `open_editor`, `next_annotation`, `prev_annotation`, `flush_output`, `toggle_collapsed`, `toggle_compact`, `toggle_wrap`, `toggle_tree`, `toggle_line_numbers`, `toggle_blame`, `toggle_word_diff`, `toggle_hunk`, `toggle_untracked`, `mark_reviewed`, `theme_select`, `filter`, `info`, `reload`, `quit`, `discard_quit`, `help`, `dismiss` Fixed modal keys (Enter, Esc in annotation/search input, confirm discard) are not remappable. Keymap-resolved actions like `open_editor` work during annotation input and can be rebound. Chord bindings do not fire during text input — use single-key `ctrl+*` bindings for actions that need to work during annotation input. diff --git a/plugins/codex/skills/revdiff/references/usage.md b/plugins/codex/skills/revdiff/references/usage.md index fe2a63fb..eadec9e3 100644 --- a/plugins/codex/skills/revdiff/references/usage.md +++ b/plugins/codex/skills/revdiff/references/usage.md @@ -35,7 +35,7 @@ When a diff contains exactly one file, revdiff automatically hides the file tree ## Markdown TOC Navigation -When reviewing a single markdown file in context-only mode (e.g., `revdiff --only=README.md`), a table-of-contents pane appears on the left listing all markdown headers with indentation by level. Use `Tab` to switch between TOC and diff, `j`/`k` to navigate headers, `n`/`p` to jump to next/prev header from either pane, `Enter` to jump to a header. The TOC highlights the current section as you scroll. Headers inside fenced code blocks are excluded. +When reviewing a single markdown file in context-only mode (e.g., `revdiff --only=README.md`), a table-of-contents pane appears on the configured tree side (left by default) listing all markdown headers with indentation by level. Use `Tab` to switch between TOC and diff, `j`/`k` to navigate headers, `n`/`p` to jump to next/prev header from either pane, `Enter` to jump to a header. The TOC highlights the current section as you scroll. Headers inside fenced code blocks are excluded. ## All-Files Mode diff --git a/plugins/pi/skills/revdiff/SKILL.md b/plugins/pi/skills/revdiff/SKILL.md index e8e0c351..97468357 100644 --- a/plugins/pi/skills/revdiff/SKILL.md +++ b/plugins/pi/skills/revdiff/SKILL.md @@ -23,6 +23,7 @@ Reference resolution rules: - For date requests, resolve the commit first. Examples: `2 weeks ago`, `yesterday`, `last Friday` → run `git rev-list -1 --before= HEAD`, then pass the resulting commit hash as `args`. - For file targets, use `args: "--only "`. - For all-files requests, map excludes explicitly. Example: `all files exclude vendor and dist` → `args: "--all-files --exclude=vendor --exclude=dist"`. +- When the user explicitly requests the file tree on a side, pass `--tree-position=left` or `--tree-position=right`; otherwise respect their config. - For explicit refs, ranges, flags, or two-ref requests, pass them through as revdiff args. - If the requested natural language target cannot be resolved, say what failed and ask for a concrete ref/path. Do not guess silently. diff --git a/site/docs.html b/site/docs.html index 4a9daf1f..1ddcbe16 100644 --- a/site/docs.html +++ b/site/docs.html @@ -289,7 +289,7 @@

Review description

--description and --description-file are mutually exclusive. The description section is hidden when neither is set, so the flag is purely additive.

Markdown TOC navigation

-

When reviewing a single markdown file in context-only mode, a table-of-contents pane appears on the left listing all markdown headers. This works for --only files and for --stdin when --stdin-name ends with .md or .markdown. Use Tab to switch focus, j/k to navigate, Enter to jump. The TOC highlights the current section as you scroll. Headers inside fenced code blocks are excluded.

+

When reviewing a single markdown file in context-only mode, a table-of-contents pane appears on the configured side listing all markdown headers. This works for --only files and for --stdin when --stdin-name ends with .md or .markdown. Use Tab to switch focus, j/k to navigate, Enter to jump. The TOC highlights the current section as you scroll. Headers inside fenced code blocks are excluded.

Beyond code review

The --only and --stdin flags enable use cases beyond git diffs. Files and ephemeral command output can both be loaded for annotation, no git repo required.

@@ -396,6 +396,7 @@

Options

--stagedShow staged changesfalse --untrackedShow untracked files in the treefalse --tree-widthFile tree width in units (1-10)2 + --tree-positionFile tree and markdown TOC position (left or right)left --tab-widthSpaces per tab character4 --no-colorsDisable all colorsfalse --no-status-barHide the status barfalse @@ -652,7 +653,7 @@

Chord bindings (ctrl/alt leader)

Available actions

Navigation: down, up, page_down, page_up, half_page_down, half_page_up, home, end, scroll_left, scroll_right, scroll_center, scroll_top, scroll_bottom, scroll_diff_down, scroll_diff_up

File/Hunk: next_item, prev_item, jump_file, next_hunk, prev_hunk, open_file_in_editor

-

Pane: toggle_pane, focus_tree, focus_diff

+

Pane: toggle_pane, focus_left, focus_right, focus_tree, focus_diff

Search: search

Annotations: confirm, annotate_file, delete_annotation, annot_list, open_editor, next_annotation, prev_annotation, flush_output

View: toggle_collapsed, toggle_compact, toggle_wrap, toggle_tree, toggle_line_numbers, toggle_blame, toggle_word_diff, toggle_hunk, toggle_untracked, mark_reviewed, theme_select, filter, info, reload