diff --git a/docs/decisions/FEAT-0014 Folder Expansion.md b/docs/decisions/FEAT-0014 Folder Expansion.md new file mode 100644 index 00000000..c42e43a0 --- /dev/null +++ b/docs/decisions/FEAT-0014 Folder Expansion.md @@ -0,0 +1,38 @@ +--- +title: Folder Expansion +description: Opt-in gitui-style folder expand/collapse on h/l in the file list, the diff follows the file-list cursor, and the sticky prefix keeps badges visible during horizontal scroll +type: adr +status: proposed +created: 2026-05-23 +--- + +# FEAT-0014 Folder Expansion + +## Context + +Default `h` / `l` are vim's character-navigation keys; in tuicr that maps to horizontal scroll of the focused panel. Some users would rather have those keys drive the file tree the way gitui does -- expand / collapse / descend / ascend. The two intents collide on the same keys, so the gitui behaviour is gated behind a config flag and off by default. + +The same audience also expects browse-as-you-go: moving the file-list cursor onto a different file scrolls the diff to that file's header, the way gitui and lazygit preview files. Without it the user has to press Enter to commit each selection, which is awkward when you can already see which file you're on. + +Long filenames in deep paths spill past the panel's right edge. The whole row including its status badge slides off under horizontal scroll, leaving the user with no idea which file they're on. The fix is to anchor the badge and indent at the left edge and slide only the name portion. + +## Decision + +When `arrow_tree_navigation = true`, `h` / `l` in the file list fall through to gitui-style tree nav at the horizontal scroll boundary: + +- `l` on a collapsed folder expands it; on an expanded folder descends to the first child; on a file jumps to the next folder below. +- `h` on an expanded folder collapses it; otherwise ascends to the parent. At the top level it jumps to the previous folder above. + +Default is `false`: `h` / `l` only scroll horizontally, vim-style. Folders are still expandable with `Enter` / `Space` regardless of the flag. + +With the flag on, moving the file-list cursor (`j` / `k`, arrows, or the tree-nav keys above) to a file also scrolls the diff to that file's header. Focus stays on the file list so the user can keep browsing. Folders are a no-op so arrowing past collapsed entries doesn't churn the diff viewport. Enter still commits a selection and shifts focus, matching the existing pattern. + +Horizontal scroll keeps the sticky prefix (indent, expand icon, checkbox, status badge) anchored at the left edge; only the filename portion slides. Scroll is capped so at least one column of the longest name stays visible. Leaving the file list resets `scroll_x` to 0 so long names re-enter from the start next time focus returns. + +## Consequences + +- [+] Vim-pure default. Users who want tree nav opt in. +- [+] Browse-as-you-go matches gitui / lazygit muscle memory. +- [+] Long filenames stay identifiable mid-scroll because the badge never disappears. +- [-] The opt-in is invisible without reading the config docs. +- [-] The diff scrolls every time the file-list cursor lands on a different file, which can be jarring if the user is mid-read. Mitigated by keeping the opt-in off by default. diff --git a/src/app.rs b/src/app.rs index 467e1567..e5dd762b 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1019,6 +1019,12 @@ pub struct App { /// Symmetric inverse of [`down_released_since_arm`] for the prev-file /// walk gate. pub up_released_since_arm: bool, + /// When true, `h`/`l` in the file list run gitui-style tree + /// navigation (expand/collapse/descend/ascend) once the panel is + /// at its horizontal scroll boundary. Defaults to false so `h`/`l` + /// stay as plain horizontal scroll, matching vim's character + /// navigation; users opt in via `arrow_tree_navigation = true`. + pub arrow_tree_navigation: bool, pub cursor_line_highlight: bool, pub leader_key: char, pub scroll_offset: usize, @@ -1125,6 +1131,13 @@ impl FileListState { let max_scroll_x = self.max_content_width.saturating_sub(self.viewport_width); self.scroll_x = (self.scroll_x.saturating_add(cols)).min(max_scroll_x); } + + /// True when the rightmost column of the rendered file list is + /// already visible. The tree-nav fallthrough on `l` triggers here. + pub fn at_max_scroll_x(&self) -> bool { + let max_scroll_x = self.max_content_width.saturating_sub(self.viewport_width); + self.scroll_x >= max_scroll_x + } } #[derive(Default)] @@ -1785,6 +1798,7 @@ impl App { primed_walk_prev: false, down_released_since_arm: false, up_released_since_arm: false, + arrow_tree_navigation: false, cursor_line_highlight: true, leader_key: crate::config::DEFAULT_LEADER_KEY, scroll_offset: 0, @@ -5409,6 +5423,19 @@ impl App { self.update_current_file_from_cursor(); } + /// Browse-as-you-go: when `arrow_tree_navigation` is on, moving the + /// file-list cursor to a file scrolls the diff to that file's header + /// without changing focus. Folders are a no-op so the user can still + /// arrow past collapsed entries while reading the diff. + pub fn auto_jump_to_selected_file_if_enabled(&mut self) { + if !self.arrow_tree_navigation { + return; + } + if let Some(FileTreeItem::File { file_idx, .. }) = self.get_selected_tree_item() { + self.jump_to_file(file_idx); + } + } + pub fn next_file(&mut self) { let visible_items = self.build_visible_items(); let current_file_idx = self.diff_state.current_file_idx; @@ -8549,6 +8576,70 @@ impl App { } } + /// Select the parent directory of the current tree selection. Returns + /// `true` if a parent existed and was selected; `false` at the root. + /// Walks up from the current item's path to the nearest visible + /// `Directory` entry in the rendered tree. + pub fn file_list_select_parent(&mut self) -> bool { + use std::path::Path; + + let item = match self.get_selected_tree_item() { + Some(i) => i, + None => return false, + }; + let child_path = match &item { + FileTreeItem::Directory { path, .. } => path.clone(), + FileTreeItem::File { file_idx, .. } => match self.diff_files.get(*file_idx) { + Some(file) => file.display_path().display().to_string(), + None => return false, + }, + }; + let parent = match Path::new(&child_path).parent() { + Some(p) if !p.as_os_str().is_empty() => p.to_string_lossy().to_string(), + _ => return false, + }; + let visible = self.build_visible_items(); + for (idx, entry) in visible.iter().enumerate() { + if let FileTreeItem::Directory { path, .. } = entry + && *path == parent + { + self.file_list_state.select(idx); + return true; + } + } + false + } + + /// Select the next visible `Directory` entry below the current + /// selection, skipping over any intervening files. Returns `true` + /// if a folder was found and selected. + pub fn file_list_select_next_folder(&mut self) -> bool { + let visible = self.build_visible_items(); + let start = self.file_list_state.selected() + 1; + for (offset, entry) in visible.iter().skip(start).enumerate() { + if matches!(entry, FileTreeItem::Directory { .. }) { + self.file_list_state.select(start + offset); + return true; + } + } + false + } + + /// Select the previous visible `Directory` entry above the current + /// selection, skipping over any intervening files. Returns `true` + /// if a folder was found and selected. + pub fn file_list_select_prev_folder(&mut self) -> bool { + let visible = self.build_visible_items(); + let current = self.file_list_state.selected(); + for idx in (0..current).rev() { + if matches!(visible.get(idx), Some(FileTreeItem::Directory { .. })) { + self.file_list_state.select(idx); + return true; + } + } + false + } + /// Get the line boundaries (start_line, end_line) of a gap. fn gap_boundaries(&self, gap_id: &GapId) -> Option<(u32, u32)> { let file = self.diff_files.get(gap_id.file_idx)?; @@ -14446,4 +14537,36 @@ mod single_file_view_tests { let current = &app.diff_files[0].clone(); assert!(app.effective_file_height(0, current) > 0); } + + #[test] + fn file_list_select_parent_jumps_from_file_to_containing_dir() { + let files = vec![file("src/a.rs", vec![hunk(1, 3)])]; + let mut app = app_with(files); + app.expanded_dirs.insert("src".to_string()); + // Visible: [Directory("src"), File("src/a.rs")] -- select the file. + app.file_list_state.select(1); + assert!(matches!( + app.get_selected_tree_item(), + Some(FileTreeItem::File { .. }) + )); + + let walked = app.file_list_select_parent(); + + assert!(walked); + assert!(matches!( + app.get_selected_tree_item(), + Some(FileTreeItem::Directory { path, .. }) if path == "src" + )); + } + + #[test] + fn file_list_select_parent_at_root_returns_false() { + let files = vec![file("root.rs", vec![hunk(1, 3)])]; + let mut app = app_with(files); + app.file_list_state.select(0); + + let walked = app.file_list_select_parent(); + + assert!(!walked); + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index 004dd521..5307ce2e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -63,6 +63,12 @@ pub struct AppConfig { /// used as the "viewer" identity for per-author coloring in the comment /// pane. Defaults to `"user"` when unset. pub username: Option, + /// When true, `h` and `l` in the file list also drive gitui-style + /// tree navigation at the scroll boundary: expand / collapse folders + /// and descend / ascend the tree. Defaults to false so `h` / `l` + /// stay as plain horizontal scroll, matching vim's character + /// navigation. Set to true to opt in. + pub arrow_tree_navigation: Option, /// `[forge]` section settings. Always present; `None` means "no override" /// and downstream code should treat it as `ForgeConfig::default()`. pub forge: Option, @@ -90,6 +96,7 @@ const KNOWN_KEYS: &[&str] = &[ "no_update_check", "single_file_view", "username", + "arrow_tree_navigation", "forge", ]; @@ -299,6 +306,7 @@ fn load_config_from_path(path: &Path) -> Result { no_update_check: read_bool(table, "no_update_check", &mut warnings), single_file_view: read_bool(table, "single_file_view", &mut warnings), username: read_string(table, "username", &mut warnings), + arrow_tree_navigation: read_bool(table, "arrow_tree_navigation", &mut warnings), forge: table .get("forge") .and_then(|v| parse_forge(v, &mut warnings)), diff --git a/src/handler.rs b/src/handler.rs index d7178ebb..5ef35405 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -1039,10 +1039,68 @@ pub fn handle_visual_action(app: &mut App, action: Action) { /// Handle actions when file list panel is focused pub fn handle_file_list_action(app: &mut App, action: Action) { match action { - Action::CursorDown(n) => app.file_list_down(n), - Action::CursorUp(n) => app.file_list_up(n), - Action::ScrollLeft(n) => app.file_list_state.scroll_left(n), - Action::ScrollRight(n) => app.file_list_state.scroll_right(n), + Action::CursorDown(n) => { + app.file_list_down(n); + app.auto_jump_to_selected_file_if_enabled(); + } + Action::CursorUp(n) => { + app.file_list_up(n); + app.auto_jump_to_selected_file_if_enabled(); + } + // h/Left: scroll the file list left while there's hidden content; + // at the leftmost column fall through to gitui-style tree nav -- + // collapse an expanded folder, ascend to the parent, or (at the + // top level with no parent) jump to the previous folder above, + // skipping over intervening files. When + // `arrow_tree_navigation = false`, the fall-through is disabled + // and only horizontal scroll remains. + Action::ScrollLeft(n) => { + if app.file_list_state.scroll_x > 0 { + app.file_list_state.scroll_left(n); + } else if app.arrow_tree_navigation { + match app.get_selected_tree_item() { + Some(FileTreeItem::Directory { ref path, .. }) + if app.expanded_dirs.contains(path) => + { + let path = path.clone(); + app.toggle_directory(&path); + } + _ => { + if !app.file_list_select_parent() { + app.file_list_select_prev_folder(); + } + } + } + app.auto_jump_to_selected_file_if_enabled(); + } + } + // l/Right: scroll the file list right while there's hidden content; + // at the rightmost column fall through to gitui-style tree nav -- + // expand a collapsed folder, descend into an expanded one, or + // (on a file) jump to the next folder below, skipping over + // intervening files. When `arrow_tree_navigation = false`, the + // fall-through is disabled. + Action::ScrollRight(n) => { + if !app.file_list_state.at_max_scroll_x() { + app.file_list_state.scroll_right(n); + } else if app.arrow_tree_navigation { + match app.get_selected_tree_item() { + Some(FileTreeItem::Directory { ref path, .. }) + if !app.expanded_dirs.contains(path) => + { + let path = path.clone(); + app.toggle_directory(&path); + } + Some(FileTreeItem::Directory { .. }) => { + app.file_list_down(1); + } + _ => { + app.file_list_select_next_folder(); + } + } + app.auto_jump_to_selected_file_if_enabled(); + } + } Action::MouseScrollDown(n) => app.file_list_viewport_scroll_down(n), Action::MouseScrollUp(n) => app.file_list_viewport_scroll_up(n), Action::SelectFile | Action::ToggleExpand => { diff --git a/src/main.rs b/src/main.rs index c28f4155..c5f072ec 100644 --- a/src/main.rs +++ b/src/main.rs @@ -297,6 +297,9 @@ fn main() -> anyhow::Result<()> { if cfg.single_file_view == Some(true) && !app.is_single_file_view { app.toggle_single_file_view(); } + if let Some(tree_nav) = cfg.arrow_tree_navigation { + app.arrow_tree_navigation = tree_nav; + } if cfg.export_legend == Some(false) { app.export_legend = false; } diff --git a/src/ui/file_list.rs b/src/ui/file_list.rs index e8c874bf..a16c90c8 100644 --- a/src/ui/file_list.rs +++ b/src/ui/file_list.rs @@ -8,9 +8,19 @@ use std::path::Path; use unicode_width::UnicodeWidthStr; use crate::app::{App, FileTreeItem, FocusedPanel}; -use crate::ui::diff_view::apply_horizontal_scroll; use crate::ui::styles; +/// Skip `cols` characters from the start of `name`. Used to slide a +/// row's filename under horizontal scroll while the sticky prefix +/// (indent, checkbox, status badge) stays at the left edge. +fn scroll_name(name: &str, cols: usize) -> String { + if cols == 0 { + name.to_string() + } else { + name.chars().skip(cols).collect() + } +} + const EXPANDED_GLYPH: &str = "\u{25bc}"; // ▼ const COLLAPSED_GLYPH: &str = "\u{25b6}"; // ▶ const REVIEWED_BOX: &str = "\u{25a3}"; // ▣ @@ -19,6 +29,13 @@ const UNREVIEWED_BOX: &str = "\u{25a2}"; // ▢ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { let focused = app.focused_panel == FocusedPanel::FileList; + // Leaving the file list resets its horizontal scroll. Long filenames + // come back into view from the start next time the user focuses the + // panel; otherwise the panel would appear "stuck" mid-scroll. + if !focused { + app.file_list_state.scroll_x = 0; + } + let title = format!( " Files \u{00b7} {}/{} ", app.reviewed_count(), @@ -34,24 +51,29 @@ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { app.file_list_inner_area = Some(inner); let visible_items = app.build_visible_items(); + // Sticky prefix (indent + icon + status badge) stays anchored at + // the left edge during horizontal scroll; only the name portion + // slides. `max_content_width` therefore tracks the widest name, not + // the widest full row, so `scroll_x` maps directly to "chars skipped + // from the start of the longest name." let max_content_width = visible_items .iter() .map(|item| match item { - FileTreeItem::Directory { path, depth, .. } => { + FileTreeItem::Directory { path, .. } => { let dir_name = Path::new(path) .file_name() .and_then(|n| n.to_str()) .unwrap_or(path); - depth * 2 + 2 + dir_name.width() + 1 + dir_name.width() + 1 } - FileTreeItem::File { file_idx, depth } => { + FileTreeItem::File { file_idx, .. } => { let file = &app.diff_files[*file_idx]; let filename = file .display_path() .file_name() .and_then(|n| n.to_str()) .unwrap_or("?"); - depth * 2 + 4 + filename.width() + filename.width() } }) .max() @@ -61,7 +83,10 @@ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { app.file_list_state.viewport_height = inner.height as usize; app.file_list_state.max_content_width = max_content_width; - let max_scroll_x = max_content_width.saturating_sub(inner.width as usize); + // Cap scroll so at least one column of the longest name stays + // visible -- there's no value in scrolling the name entirely off + // screen. + let max_scroll_x = max_content_width.saturating_sub(1); if app.file_list_state.scroll_x > max_scroll_x { app.file_list_state.scroll_x = max_scroll_x; } @@ -106,10 +131,11 @@ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { .file_name() .and_then(|n| n.to_str()) .unwrap_or(path); + let name = format!("{dir_name}/"); Line::from(vec![ Span::raw(indent), Span::styled(format!("{icon} "), styles::dir_icon_style(&app.theme)), - Span::raw(format!("{dir_name}/")), + Span::raw(scroll_name(&name, scroll_x)), ]) } FileTreeItem::File { file_idx, depth } => { @@ -148,13 +174,13 @@ pub(super) fn render_file_list(frame: &mut Frame, app: &mut App, area: Rect) { styles::file_status_style(&app.theme, status), )); } - spans.push(Span::raw(filename.to_string())); + spans.push(Span::raw(scroll_name(filename, scroll_x))); Line::from(spans) } } }; - ListItem::new(apply_horizontal_scroll(line, scroll_x)) + ListItem::new(line) }) .collect();