From 4445f76cb959b7fe0dca8f76638d69399aa9164a Mon Sep 17 00:00:00 2001 From: altsem Date: Thu, 13 Nov 2025 18:34:29 +0100 Subject: [PATCH 1/2] feat: word-wrapping - update screen to be able to navigate with multi-line items - change screen mod to orient around items instead of lines - bug-fixes and support for x/y fill (previously called `grow`) in layout mod --- src/app.rs | 4 +- src/items.rs | 5 +- src/ops/editor.rs | 2 +- src/screen/mod.rs | 394 ++++++++------ .../gitu__tests__branch__checkout_picker.snap | 2 +- .../gitu__tests__branch__delete_picker.snap | 2 +- .../gitu__tests__branch__rename_picker.snap | 2 +- ..._spinoff_branch_with_unmerged_commits.snap | 4 +- ...ests__cherry_pick__cherry_pick_prompt.snap | 2 +- .../gitu__tests__commit__commit_extend.snap | 6 +- ...__tests__commit__commit_instant_fixup.snap | 6 +- ...fixup_stashes_changes_and_keeps_empty.snap | 6 +- ...tests__discard__discard_unstaged_line.snap | 6 +- ...ts__editor__re_enter_picker_from_menu.snap | 2 +- .../gitu__tests__log__log_other_invalid.snap | 6 +- .../gitu__tests__merge__merge_picker.snap | 2 +- ...sts__merge__merge_picker_custom_input.snap | 2 +- ...ests__rebase__rebase_elsewhere_prompt.snap | 2 +- .../gitu__tests__rebase__rebase_menu.snap | 18 +- ...tests__stage_last_hunk_of_first_delta.snap | 6 +- ...u__tests__unstage__unstage_added_line.snap | 6 +- src/ui.rs | 44 +- src/ui/layout/mod.rs | 492 ++++++++++++------ src/ui/layout/node.rs | 78 ++- src/ui/menu.rs | 25 +- src/ui/picker.rs | 5 +- 26 files changed, 738 insertions(+), 391 deletions(-) diff --git a/src/app.rs b/src/app.rs index 6c1afe3f05..a9d42b39ed 100644 --- a/src/app.rs +++ b/src/app.rs @@ -195,7 +195,7 @@ impl App { pub fn update_screens(&mut self) -> Res<()> { for screen in &mut self.state.screens { - screen.update()?; + screen.refresh()?; } self.stage_redraw(); @@ -206,7 +206,7 @@ impl App { match event { Event::Resize(w, h) => { for screen in self.state.screens.iter_mut() { - screen.size = Size::new(w, h); + screen.resize(w, h)?; } self.stage_redraw(); diff --git a/src/items.rs b/src/items.rs index c4dbbc82bf..15860260b7 100644 --- a/src/items.rs +++ b/src/items.rs @@ -18,7 +18,6 @@ use std::hash::Hash; use std::hash::Hasher; use std::iter; use std::rc::Rc; -use std::sync::Arc; pub type ItemId = u64; @@ -32,7 +31,7 @@ pub(crate) struct Item { } impl Item { - pub fn to_line(&'_ self, config: Arc) -> Line<'_> { + pub fn to_line(&'_ self, config: &Config) -> Line<'_> { match self.data.clone() { ItemData::Raw(content) => Line::raw(content), ItemData::AllUnstaged(count) => Line::from(vec![ @@ -118,7 +117,7 @@ impl Item { line_i, } => { let hunk_highlights = - highlight::highlight_hunk(self.id, &config, &Rc::clone(&diff), file_i, hunk_i); + highlight::highlight_hunk(self.id, config, &Rc::clone(&diff), file_i, hunk_i); let hunk_content = &diff.hunk_content(file_i, hunk_i); let hunk_line = &hunk_content[line_range.clone()]; diff --git a/src/ops/editor.rs b/src/ops/editor.rs index 500445c6c2..2ec1547292 100644 --- a/src/ops/editor.rs +++ b/src/ops/editor.rs @@ -133,7 +133,7 @@ impl OpTrait for ToggleSection { fn get_action(&self, target: &ItemData) -> Option { if target.is_section() { Some(Rc::new(|app, _term| { - app.screen_mut().toggle_section(); + app.screen_mut().toggle_section()?; Ok(()) })) } else { diff --git a/src/screen/mod.rs b/src/screen/mod.rs index d7ded5a596..ebda1c1166 100644 --- a/src/screen/mod.rs +++ b/src/screen/mod.rs @@ -1,15 +1,15 @@ use crate::config::StyleConfig; -use crate::ui::layout::OPTS; -use crate::ui::{UiTree, layout_span}; +use crate::ui::layout::{LayoutTree, OPTS}; +use crate::ui::{UiItem, UiTree, layout_span}; use crate::{item_data::ItemData, ui}; -use ratatui::{layout::Size, style::Style, text::Line}; -use unicode_segmentation::UnicodeSegmentation; +use itertools::Itertools; +use ratatui::{layout::Size, style::Style}; use crate::{Res, config::Config, items::hash}; use super::Item; use std::borrow::Cow; -use std::collections::HashSet; +use std::collections::{BTreeMap, BTreeSet, HashSet}; use std::sync::Arc; pub(crate) mod blame; @@ -35,7 +35,13 @@ pub(crate) struct Screen { config: Arc, refresh_items: Box Res>>, items: Vec, + /// Set of item (by their index in `items`) that are not collapsed + expanded_items: BTreeSet, + /// Maps line -> item, items spanning multiple lines appear as duplicates line_index: Vec, + /// Maps line -> item, but only their first line + unique_line_index: BTreeMap, + item_heights: Vec, collapsed: HashSet, } @@ -60,11 +66,14 @@ impl Screen { config, refresh_items, items: vec![], + expanded_items: BTreeSet::new(), line_index: vec![], + unique_line_index: BTreeMap::new(), + item_heights: vec![], collapsed, }; - screen.update()?; + screen.refresh()?; // TODO Maybe this should be done on update. Better keep track of toggled sections rather than collapsed then. screen @@ -74,25 +83,18 @@ impl Screen { .for_each(|item| { screen.collapsed.insert(item.id); }); - screen.update_line_index(); + screen.update_indices()?; screen.cursor = screen .find_first_hunk() - .or_else(|| screen.find_first_selectable()) + .or_else(|| screen.find_item(|item| !item.unselectable)) .unwrap_or(0); Ok(screen) } fn find_first_hunk(&mut self) -> Option { - (0..self.line_index.len()).find(|&line_i| { - !self.at_line(line_i).unselectable - && matches!(self.at_line(line_i).data, ItemData::Hunk { .. }) - }) - } - - fn find_first_selectable(&mut self) -> Option { - (0..self.line_index.len()).find(|&line_i| !self.at_line(line_i).unselectable) + self.find_item(|item| !item.unselectable && matches!(item.data, ItemData::Hunk { .. })) } fn at_line(&self, line_i: usize) -> &Item { @@ -106,28 +108,40 @@ impl Screen { } fn scroll_fit_start(&mut self) { - if self.items.is_empty() { + if self.line_index.is_empty() { return; } - - let top = self.cursor.saturating_sub(self.get_selected_item().depth); + let Some(line_of_item) = self.line_of_item(self.cursor) else { + return; + }; + let top = line_of_item.saturating_sub(self.get_selected_item().depth); if top < self.scroll { self.scroll = top; } } fn scroll_fit_end(&mut self) { - if self.items.is_empty() { + if self.line_index.is_empty() { return; } let depth = self.get_selected_item().depth; + let current_item_i = self.cursor; + + let Some(line_of_item) = self.line_of_item(self.cursor) else { + return; + }; + + let Some(last_item_line) = (line_of_item..self.line_index.len()) + .take_while(|&line_i| { + self.line_index[line_i] == current_item_i || depth < self.at_line(line_i).depth + }) + .last() + else { + return; + }; - let last = BOTTOM_CONTEXT_LINES - + (self.cursor..self.line_index.len()) - .take_while(|&line_i| line_i == self.cursor || depth < self.at_line(line_i).depth) - .last() - .unwrap(); + let last = BOTTOM_CONTEXT_LINES + last_item_line; let end_line = self.size.height.saturating_sub(1) as usize; if last > end_line + self.scroll { @@ -136,14 +150,18 @@ impl Screen { } pub(crate) fn find_next(&mut self, nav_mode: NavMode) -> usize { - (self.cursor..self.line_index.len()) + (self.cursor..self.items.len()) .skip(1) - .find(|&line_i| self.nav_filter(line_i, nav_mode)) + .find(|&item_i| self.nav_filter(item_i, nav_mode)) .unwrap_or(self.cursor) } - fn nav_filter(&self, line_i: usize, nav_mode: NavMode) -> bool { - let item = self.at_line(line_i); + fn nav_filter(&self, item_i: usize, nav_mode: NavMode) -> bool { + if !self.expanded_items.contains(&item_i) { + return false; + } + + let item = &self.items[item_i]; match nav_mode { NavMode::Normal => { let is_sub_line = matches!( @@ -166,8 +184,7 @@ impl Screen { fn find_previous(&mut self, nav_mode: NavMode) -> usize { (0..self.cursor) - .rev() - .find(|&line_i| self.nav_filter(line_i, nav_mode)) + .rfind(|&item_i| self.nav_filter(item_i, nav_mode)) .unwrap_or(self.cursor) } @@ -191,8 +208,8 @@ impl Screen { self.clamp_scroll(); } - pub(crate) fn toggle_section(&mut self) { - let selected = &self.items[self.line_index[self.cursor]]; + pub(crate) fn toggle_section(&mut self) -> Res<()> { + let selected = &self.items[self.cursor]; if selected.data.is_section() { if self.collapsed.contains(&selected.id) { @@ -202,18 +219,25 @@ impl Screen { } } - self.update_line_index(); + self.update_indices()?; + Ok(()) } - pub(crate) fn update(&mut self) -> Res<()> { - let nav_mode = self.selected_item_nav_mode(); + pub(crate) fn refresh(&mut self) -> Res<()> { self.items = (self.refresh_items)()?; - self.update_line_index(); - self.update_cursor(nav_mode); + self.update_indices()?; + self.update_cursor(); Ok(()) } - fn update_cursor(&mut self, nav_mode: NavMode) { + pub(crate) fn resize(&mut self, w: u16, h: u16) -> Res<()> { + self.size = Size::new(w, h); + self.update_indices()?; + self.update_cursor(); + Ok(()) + } + + fn update_cursor(&mut self) { // Nothing is selectable (e.g. the log of a branch with no commits). // Reset the cursor to a valid sentinel rather than positioning it, // which would index into an empty `line_index` and panic (#262). @@ -229,6 +253,7 @@ impl Screen { } self.clamp_cursor(); + let nav_mode = self.selected_item_nav_mode(); self.move_from_unselectable(nav_mode); } @@ -243,9 +268,63 @@ impl Screen { } } - fn update_line_index(&mut self) { + pub(crate) fn update_indices(&mut self) -> Res<()> { + self.update_item_heights(); + + assert_eq!( + self.items.len(), + self.item_heights.len(), + "items and item_heights should have equal len" + ); + self.line_index = self - .items + .filter_collapsed_items(&self.items) + .flat_map(|(i, _item)| [i].repeat(self.item_heights[i] as usize)) + .collect(); + + self.unique_line_index = self + .line_index + .iter() + .cloned() + .enumerate() + .unique_by(|&(_, v)| v) + .collect(); + + self.expanded_items = self + .filter_collapsed_items(&self.items) + .map(|(i, _)| i) + .collect(); + + self.clamp_scroll(); + Ok(()) + } + + fn update_item_heights(&mut self) { + self.item_heights = (0..self.items.len()) + .map(|item_index| { + let mut layout = LayoutTree::new(); + let view = ItemView { + item_index, + highlighted: false, + }; + layout_item(&mut layout, self, false, view); + layout.compute([self.size.width, self.size.height]); + + layout + .iter() + .map(|item| item.pos[1] + item.size[1]) + .max() + .unwrap_or(0) + }) + .collect() + } + + // FIXME Need to consider this when navigating + fn filter_collapsed_items<'a>( + &'a self, + items: &'a [Item], + ) -> impl Iterator { + items .iter() .enumerate() .scan(None, |collapse_depth, (i, next)| { @@ -262,24 +341,19 @@ impl Screen { Some(Some((i, next))) }) .flatten() - .map(|(i, _item)| i) - .collect(); - self.clamp_scroll(); } fn is_cursor_off_screen(&self) -> bool { - !self.line_views(self.size).any(|line| line.highlighted) + !self.item_views(self.size).any(|item| item.highlighted) } fn move_cursor_to_screen_center(&mut self) { let half_screen = self.size.height as usize / 2; - self.cursor = self.scroll + half_screen; + self.cursor = self.line_index[self.scroll + half_screen]; } fn clamp_cursor(&mut self) { - self.cursor = self - .cursor - .clamp(0, self.line_index.len().saturating_sub(1)); + self.cursor = self.cursor.clamp(0, self.items.len().saturating_sub(1)); } fn clamp_scroll(&mut self) { @@ -312,12 +386,10 @@ impl Screen { } pub(crate) fn move_cursor_to_screen_line(&mut self, screen_line: usize) { - if self.line_index.is_empty() { + let Some(&new_cursor) = self.line_index.get(screen_line + self.scroll) else { return; - } - - let new_cursor = screen_line + self.scroll; - if new_cursor >= self.line_index.len() || self.cursor == new_cursor { + }; + if self.cursor == new_cursor { return; } @@ -337,48 +409,48 @@ impl Screen { } pub(crate) fn move_cursor_to_top(&mut self) { - if self.line_index.is_empty() { + if self.unique_line_index.is_empty() { return; } - if let Some(first) = self.find_first_selectable() { + if let Some(first) = self.find_item(|item| !item.unselectable) { self.cursor = first; self.scroll = 0; } } pub(crate) fn move_cursor_to_bottom(&mut self) { - if self.line_index.is_empty() { + if self.unique_line_index.is_empty() { return; } - if let Some(last) = self.find_last_selectable() { + if let Some(last) = self.rfind_item(|item| !item.unselectable) { self.cursor = last; self.scroll_fit_end(); } } - fn find_last_selectable(&self) -> Option { - (0..self.line_index.len()).rfind(|&line_i| !self.at_line(line_i).unselectable) - } - pub(crate) fn is_collapsed(&self, item: &Item) -> bool { self.collapsed.contains(&item.id) } pub(crate) fn get_selected_item(&self) -> &Item { - &self.items[self.line_index[self.cursor]] + &self.items[self.cursor] } pub(crate) fn select_matching bool>(&mut self, predicate: F) -> bool { - if let Some(line_i) = (0..self.line_index.len()).find(|&line_i| { - !self.at_line(line_i).unselectable && predicate(&self.at_line(line_i).data) - }) { - self.cursor = line_i; + if let Some(item_i) = self.find_item(|item| !item.unselectable && predicate(&item.data)) { + self.cursor = item_i; let half_screen = self.size.height as usize / 2; - if self.cursor >= half_screen { - self.scroll = self.cursor - half_screen; + let Some(line_of_item) = self.line_of_item(self.cursor) else { + return false; + }; + + if line_of_item >= half_screen { + self.scroll = line_of_item - half_screen; } + self.scroll_fit_end(); self.scroll_fit_start(); + true } else { false @@ -386,130 +458,132 @@ impl Screen { } pub(crate) fn select_last_matching bool>(&mut self, predicate: F) -> bool { - if let Some(line_i) = (0..self.line_index.len()).rev().find(|&line_i| { - !self.at_line(line_i).unselectable && predicate(&self.at_line(line_i).data) - }) { - self.cursor = line_i; + if let Some(item_i) = self.rfind_item(|item| !item.unselectable && predicate(&item.data)) { + self.cursor = item_i; let half_screen = self.size.height as usize / 2; - if self.cursor >= half_screen { - self.scroll = self.cursor - half_screen; + let Some(line_of_item) = self.line_of_item(self.cursor) else { + return false; + }; + + if line_of_item >= half_screen { + self.scroll = line_of_item - half_screen; } else { self.scroll_fit_start(); } + true } else { false } } + fn find_item bool>(&self, predicate: P) -> Option { + self.unique_line_index + .iter() + .find(|&(_, &item_i)| predicate(&self.items[item_i])) + .map(|(_, &item_i)| item_i) + } + + fn rfind_item bool>(&self, predicate: P) -> Option { + self.unique_line_index + .iter() + .rfind(|&(_, &item_i)| predicate(&self.items[item_i])) + .map(|(_, &item_i)| item_i) + } + pub(crate) fn is_valid_screen_line(&self, screen_line: usize) -> bool { - let target_line_i = screen_line + self.scroll; - if self.line_index.is_empty() || target_line_i >= self.line_index.len() { + let Some(target_item_i) = self.line_of_item(screen_line + self.scroll) else { return false; - } - self.nav_filter(target_line_i, NavMode::IncludeSubLines) + }; + self.nav_filter(target_item_i, NavMode::IncludeSubLines) } - fn line_views(&'_ self, area: Size) -> impl Iterator> { - let scan_start = self.scroll.min(self.cursor); - let scan_end = (self.scroll + area.height as usize).min(self.line_index.len()); - let scan_highlight_range = scan_start..(scan_end); - let context_lines = self.scroll - scan_start; - - self.line_index[scan_highlight_range] + fn line_of_item(&self, item_i: usize) -> Option { + self.unique_line_index .iter() - .scan(None, |highlight_depth, item_index| { - let item = &self.items[*item_index]; - if self.line_index[self.cursor] == *item_index { + .find(|&(_, &i)| item_i == i) + .map(|(&line_i, _)| line_i) + } + + fn item_views(&'_ self, area: Size) -> impl Iterator { + let first_visible_item = self + .line_index + .get(self.scroll) + .cloned() + .unwrap_or(self.items.len().saturating_sub(1)); + + let scan_start_item = first_visible_item.min(self.cursor); + let scan_end_line = (self.scroll + area.height as usize).min(self.line_index.len()); + let scan_end_item = self + .line_index + .get(scan_end_line) + .cloned() + .unwrap_or(self.items.len()); + + let scan_highlight_range = scan_start_item..(scan_end_item); + let context_offset = self + .expanded_items + .range(scan_start_item..first_visible_item) + .count(); + + self.filter_collapsed_items(&self.items[scan_highlight_range]) + .scan(None, move |highlight_depth, (offset_item_index, item)| { + let item_index = scan_start_item + offset_item_index; + if self.cursor == item_index { *highlight_depth = Some(item.depth); } else if highlight_depth.is_some_and(|s| s >= item.depth) { *highlight_depth = None; }; - let display = item.to_line(Arc::clone(&self.config)); - Some(LineView { - item_index: *item_index, - display, + Some(ItemView { + item_index, highlighted: highlight_depth.is_some(), }) }) - .skip(context_lines) + .skip(context_offset) } } -struct LineView<'a> { +struct ItemView { item_index: usize, - display: Line<'a>, highlighted: bool, } -const SPACES: &str = " "; +pub(crate) fn layout_screen<'a>(layout: &mut UiTree<'a>, screen: &'a Screen, hide_cursor: bool) { + layout.vertical(None, OPTS.fill_x(), |layout| { + for view in screen.item_views(screen.size) { + layout_item(layout, screen, hide_cursor, view); + } + }); +} -pub(crate) fn layout_screen<'a>( - layout: &mut UiTree<'a>, - size: Size, - screen: &'a Screen, - hide_cursor: bool, -) { +fn layout_item<'a>(layout: &mut UiTree<'a>, screen: &'a Screen, hide_cursor: bool, line: ItemView) { let style = &screen.config.style; + let is_line_sel = screen.cursor == line.item_index; - layout.vertical(None, OPTS, |layout| { - for line in screen.line_views(size) { - layout.horizontal(None, OPTS, |layout| { - let is_line_sel = screen.line_index[screen.cursor] == line.item_index; - let area_sel = area_selection_highlight(style, &line); - let line_sel = line_selection_highlight(style, &line, is_line_sel); - let bg = area_sel.patch(line_sel); - - let mut line_end = 1; - let gutter_char = if !hide_cursor && line.highlighted { - gutter_char(style, is_line_sel, bg) - } else { - (" ".into(), Style::new()) - }; + let area_sel = area_selection_highlight(style, &line); + let line_sel = line_selection_highlight(style, &line, is_line_sel); + let bg = area_sel.patch(line_sel); - layout_span(layout, gutter_char); - - line.display.spans.into_iter().for_each(|span| { - let style = bg.patch(line.display.style).patch(span.style); - - let span_width = span.content.graphemes(true).count(); - - if line_end + span_width >= size.width as usize { - // Truncate the span and insert an ellipsis to indicate overflow - let overflow = line_end + span_width - size.width as usize; - line_end = size.width as usize; - ui::layout_span( - layout, - ( - span.content - .graphemes(true) - .take(span_width.saturating_sub(overflow + 1)) - .collect::() - .into(), - style, - ), - ); - layout_span(layout, ("…".into(), bg)); - } else { - // Insert the span as normal - line_end += span_width; - ui::layout_span(layout, (span.content, style)); - } - }); - - // Add ellipsis indicator for collapsed sections - let item = &screen.items[line.item_index]; - if screen.is_collapsed(item) { - line_end += 1; - layout_span(layout, ("…".into(), bg)); - } + layout.horizontal(Some(UiItem::Style(bg)), OPTS.fill_x(), |layout| { + let gutter_char = if !hide_cursor && line.highlighted { + gutter_char(style, is_line_sel, bg) + } else { + (" ".into(), Style::new()) + }; - // Style the rest of the line's empty space - let style = if is_line_sel { line_sel } else { area_sel }; - let padding_width = (size.width as usize).saturating_sub(line_end); - ui::repeat_chars(layout, padding_width, SPACES, style); - }); + layout_span(layout, gutter_char); + + let item_line = screen.items[line.item_index].to_line(&screen.config); + item_line.spans.into_iter().for_each(|span| { + let style = bg.patch(item_line.style).patch(span.style); + ui::layout_span(layout, (span.content, style)); + }); + + // Add ellipsis indicator for collapsed sections + let item = &screen.items[line.item_index]; + if screen.is_collapsed(item) { + layout_span(layout, ("…".into(), bg)); } }); } @@ -528,7 +602,7 @@ fn gutter_char<'a>(style: &'a StyleConfig, is_line_sel: bool, bg: Style) -> (Cow } } -fn line_selection_highlight(style: &StyleConfig, line: &LineView, selected_line: bool) -> Style { +fn line_selection_highlight(style: &StyleConfig, line: &ItemView, selected_line: bool) -> Style { if line.highlighted && selected_line { Style::from(&style.selection_line) } else { @@ -536,7 +610,7 @@ fn line_selection_highlight(style: &StyleConfig, line: &LineView, selected_line: } } -fn area_selection_highlight(style: &StyleConfig, line: &LineView) -> Style { +fn area_selection_highlight(style: &StyleConfig, line: &ItemView) -> Style { if line.highlighted { Style::from(&style.selection_area) } else { diff --git a/src/tests/snapshots/gitu__tests__branch__checkout_picker.snap b/src/tests/snapshots/gitu__tests__branch__checkout_picker.snap index 62c87e1e2a..1918f6c19c 100644 --- a/src/tests/snapshots/gitu__tests__branch__checkout_picker.snap +++ b/src/tests/snapshots/gitu__tests__branch__checkout_picker.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: d0729ae36786267b +styles_hash: 58f7548685b20bd5 diff --git a/src/tests/snapshots/gitu__tests__branch__delete_picker.snap b/src/tests/snapshots/gitu__tests__branch__delete_picker.snap index c86d5923bb..280a39699e 100644 --- a/src/tests/snapshots/gitu__tests__branch__delete_picker.snap +++ b/src/tests/snapshots/gitu__tests__branch__delete_picker.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: 6a255f9ee8d30c59 +styles_hash: 45fbe6e2f998d7e4 diff --git a/src/tests/snapshots/gitu__tests__branch__rename_picker.snap b/src/tests/snapshots/gitu__tests__branch__rename_picker.snap index 14a18c0e0f..8d1582fec8 100644 --- a/src/tests/snapshots/gitu__tests__branch__rename_picker.snap +++ b/src/tests/snapshots/gitu__tests__branch__rename_picker.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: 8f7a659399e0489e +styles_hash: 67c9ab69126a0c9c diff --git a/src/tests/snapshots/gitu__tests__branch__spinoff_branch_with_unmerged_commits.snap b/src/tests/snapshots/gitu__tests__branch__spinoff_branch_with_unmerged_commits.snap index d4d00af7a6..dd6972459c 100644 --- a/src/tests/snapshots/gitu__tests__branch__spinoff_branch_with_unmerged_commits.snap +++ b/src/tests/snapshots/gitu__tests__branch__spinoff_branch_with_unmerged_commits.snap @@ -16,10 +16,10 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git checkout -b new | Switched to a new branch 'new' | $ git update-ref -m "reset: moving to b66a0bf82020d6a386e94d0fceedec1f817d20c7" | +refs/heads/main b66a0bf82020d6a386e94d0fceedec1f817d20c7 | > Branch main was reset to b66a0bf82020d6a386e94d0fceedec1f817d20c7 | -styles_hash: 3b16474e21a1768a +styles_hash: f1c23e60f9d29904 diff --git a/src/tests/snapshots/gitu__tests__cherry_pick__cherry_pick_prompt.snap b/src/tests/snapshots/gitu__tests__cherry_pick__cherry_pick_prompt.snap index bd64720487..9e2369e79e 100644 --- a/src/tests/snapshots/gitu__tests__cherry_pick__cherry_pick_prompt.snap +++ b/src/tests/snapshots/gitu__tests__cherry_pick__cherry_pick_prompt.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: f868f120e27e8b1b +styles_hash: 23f47fa15ae9f751 diff --git a/src/tests/snapshots/gitu__tests__commit__commit_extend.snap b/src/tests/snapshots/gitu__tests__commit__commit_extend.snap index b417da9858..f89da3d87e 100644 --- a/src/tests/snapshots/gitu__tests__commit__commit_extend.snap +++ b/src/tests/snapshots/gitu__tests__commit__commit_extend.snap @@ -3,7 +3,8 @@ source: src/tests/commit.rs expression: ctx.redact_buffer() --- ▌On branch main | -▌Your branch and 'origin/main' have diverged,and have 1 and 1 different commits…| +▌Your branch and 'origin/main' have diverged,and have 1 and 1 different commits | +each, respectively. | | Recent commits | 5dfe782 main add initial-file | @@ -19,7 +20,6 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git commit --amend --no-edit | -styles_hash: 33ce7d91041bde51 +styles_hash: 289029b9617670c2 diff --git a/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup.snap b/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup.snap index 68520b2e6f..a3f92adca4 100644 --- a/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup.snap +++ b/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup.snap @@ -15,11 +15,11 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git commit --fixup efc77f3bea683ce4ea27f2e9d7d1bdf04c91a57f | [main eac3fc0] fixup! modify instant_fixup.txt | Author: Author Name | 1 file changed, 1 insertion(+), 1 deletion(-) | -$ git rebase -i -q --autostash --keep-empty --autosquash efc77f3bea683ce4ea27f2e| -styles_hash: 6969ae121e8e1f63 +$ git rebase -i -q --autostash --keep-empty --autosquash | +efc77f3bea683ce4ea27f2e9d7d1bdf04c91a57f^ | +styles_hash: 6fb28ebf48baeb5e diff --git a/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup_stashes_changes_and_keeps_empty.snap b/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup_stashes_changes_and_keeps_empty.snap index d62040a827..9b411fc69e 100644 --- a/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup_stashes_changes_and_keeps_empty.snap +++ b/src/tests/snapshots/gitu__tests__commit__commit_instant_fixup_stashes_changes_and_keeps_empty.snap @@ -13,13 +13,13 @@ expression: ctx.redact_buffer() bada738 main empty commit | 2809bd7 modify instant_fixup.txt | fa09c62 add instant_fixup.txt | -▌b66a0bf origin/main add initial-file | ────────────────────────────────────────────────────────────────────────────────| $ git commit --fixup efc77f3bea683ce4ea27f2e9d7d1bdf04c91a57f | [main bec1be7] fixup! modify instant_fixup.txt | Author: Author Name | 1 file changed, 1 insertion(+), 1 deletion(-) | -$ git rebase -i -q --autostash --keep-empty --autosquash efc77f3bea683ce4ea27f2e| +$ git rebase -i -q --autostash --keep-empty --autosquash | +efc77f3bea683ce4ea27f2e9d7d1bdf04c91a57f^ | Applied autostash. | Created autostash: d682ced | -styles_hash: 22852fd934c64bf +styles_hash: ccd126e002cf43b4 diff --git a/src/tests/snapshots/gitu__tests__discard__discard_unstaged_line.snap b/src/tests/snapshots/gitu__tests__discard__discard_unstaged_line.snap index dfae36aa93..2e8495c68c 100644 --- a/src/tests/snapshots/gitu__tests__discard__discard_unstaged_line.snap +++ b/src/tests/snapshots/gitu__tests__discard__discard_unstaged_line.snap @@ -7,8 +7,8 @@ expression: ctx.redact_buffer() | Unstaged changes (1) | modified file-one | - @@ -1,2 +1 @@ | - FOO | +▌@@ -1,2 +1 @@ | +▌ FOO | ▌-BAR | | Recent commits | @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | ────────────────────────────────────────────────────────────────────────────────| $ git apply --reverse --recount | -styles_hash: baa3baebf7866985 +styles_hash: 6795b03f13817018 diff --git a/src/tests/snapshots/gitu__tests__editor__re_enter_picker_from_menu.snap b/src/tests/snapshots/gitu__tests__editor__re_enter_picker_from_menu.snap index eea969d927..eef5ce06b2 100644 --- a/src/tests/snapshots/gitu__tests__editor__re_enter_picker_from_menu.snap +++ b/src/tests/snapshots/gitu__tests__editor__re_enter_picker_from_menu.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: 49e995be810c0237 +styles_hash: f7e9b36bbf7e3d91 diff --git a/src/tests/snapshots/gitu__tests__log__log_other_invalid.snap b/src/tests/snapshots/gitu__tests__log__log_other_invalid.snap index 9f091e5705..b511e0f58c 100644 --- a/src/tests/snapshots/gitu__tests__log__log_other_invalid.snap +++ b/src/tests/snapshots/gitu__tests__log__log_other_invalid.snap @@ -19,7 +19,7 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| -! Couldn't find git revision: failed to parse revision specifier - Invalid patte| -styles_hash: b15bdaeca3de7005 +! Couldn't find git revision: failed to parse revision specifier - Invalid | +pattern ' '; class=Invalid (3); code=InvalidSpec (-12) | +styles_hash: db8d105d97033047 diff --git a/src/tests/snapshots/gitu__tests__merge__merge_picker.snap b/src/tests/snapshots/gitu__tests__merge__merge_picker.snap index aef0a95396..2fb83c7041 100644 --- a/src/tests/snapshots/gitu__tests__merge__merge_picker.snap +++ b/src/tests/snapshots/gitu__tests__merge__merge_picker.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: 16f8dd2fc9f8bc2f +styles_hash: 6960e24f02662545 diff --git a/src/tests/snapshots/gitu__tests__merge__merge_picker_custom_input.snap b/src/tests/snapshots/gitu__tests__merge__merge_picker_custom_input.snap index ac89e1a23f..59bc7f381f 100644 --- a/src/tests/snapshots/gitu__tests__merge__merge_picker_custom_input.snap +++ b/src/tests/snapshots/gitu__tests__merge__merge_picker_custom_input.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: 6f95b383fced9e3c +styles_hash: 921dd6ef05db5d3c diff --git a/src/tests/snapshots/gitu__tests__rebase__rebase_elsewhere_prompt.snap b/src/tests/snapshots/gitu__tests__rebase__rebase_elsewhere_prompt.snap index d835e436a7..3bd6121efb 100644 --- a/src/tests/snapshots/gitu__tests__rebase__rebase_elsewhere_prompt.snap +++ b/src/tests/snapshots/gitu__tests__rebase__rebase_elsewhere_prompt.snap @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | | | -styles_hash: 65f4fc0ca3a35c1e +styles_hash: 6679953df170d9fd diff --git a/src/tests/snapshots/gitu__tests__rebase__rebase_menu.snap b/src/tests/snapshots/gitu__tests__rebase__rebase_menu.snap index beb539ba93..3085f793c6 100644 --- a/src/tests/snapshots/gitu__tests__rebase__rebase_menu.snap +++ b/src/tests/snapshots/gitu__tests__rebase__rebase_menu.snap @@ -7,19 +7,19 @@ expression: ctx.redact_buffer() Recent commits | b66a0bf other-branch origin/main add initial-file | | - | - | - | - | - | - | ────────────────────────────────────────────────────────────────────────────────| Rebase Arguments | a abort -a Autosquash (--autosquash) | c continue -A Autostash (--autostash) | - e onto elsewhere -d Lie about committer date (--committer-date-is-author-| - q/esc Quit/Close -i Interactive (--interactive) | + e onto elsewhere -d Lie about committer date (--committer-date-is-author | + q/esc Quit/Close -date) | + -i Interactive (--interactive) | -k Keep empty commits (--keep-empty) | -h Disable hooks (--no-verify) | -p Preserve merges (--preserve-merges) | -styles_hash: c2a795da9f9636c1 + | + | + | + | + | +styles_hash: 3f2fe756e501a308 diff --git a/src/tests/snapshots/gitu__tests__stage_last_hunk_of_first_delta.snap b/src/tests/snapshots/gitu__tests__stage_last_hunk_of_first_delta.snap index 77c2e697eb..09eb8ba383 100644 --- a/src/tests/snapshots/gitu__tests__stage_last_hunk_of_first_delta.snap +++ b/src/tests/snapshots/gitu__tests__stage_last_hunk_of_first_delta.snap @@ -6,12 +6,12 @@ expression: ctx.redact_buffer() Your branch is ahead of 'origin/main' by 2 commit(s). | | Unstaged changes (1) | -▌modified file-two… | + modified file-two… | | Staged changes (1) | modified file-one | @@ -1,2 +1 @@ | - -asdf | +▌-asdf | blahonga | | Recent commits | @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() | ────────────────────────────────────────────────────────────────────────────────| $ git apply --cached | -styles_hash: ec215853d0f68a99 +styles_hash: 188769995793700 diff --git a/src/tests/snapshots/gitu__tests__unstage__unstage_added_line.snap b/src/tests/snapshots/gitu__tests__unstage__unstage_added_line.snap index 83c404649a..3bbb180a59 100644 --- a/src/tests/snapshots/gitu__tests__unstage__unstage_added_line.snap +++ b/src/tests/snapshots/gitu__tests__unstage__unstage_added_line.snap @@ -7,9 +7,9 @@ expression: ctx.redact_buffer() | Unstaged changes (1) | modified firstfile | - @@ -1 +1,2 @@ | +▌@@ -1 +1,2 @@ | ▌+weehooo | - blrergh | +▌ blrergh | | Staged changes (1) | modified firstfile | @@ -22,4 +22,4 @@ expression: ctx.redact_buffer() 223428c main add firstfile | ────────────────────────────────────────────────────────────────────────────────| $ git apply --cached --reverse --recount | -styles_hash: da335a72680b4013 +styles_hash: 61980d64f9b4d1c2 diff --git a/src/ui.rs b/src/ui.rs index 6a8a5426b0..b1cab4eb8f 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -18,16 +18,21 @@ pub mod picker; const CARET: &str = "\u{2588}"; const DASHES: &str = "────────────────────────────────────────────────────────────────"; -pub(crate) type UiTree<'a> = LayoutTree<(Cow<'a, str>, Style)>; +#[derive(Debug, Clone)] +pub(crate) enum UiItem<'a> { + Span(Cow<'a, str>, Style), + Style(Style), +} +pub(crate) type UiTree<'a> = LayoutTree>; pub(crate) fn ui(frame: &mut Frame, state: &mut State) { let size = frame.area().as_size(); let mut layout = UiTree::new(); layout.vertical(None, OPTS, |layout| { - layout.vertical(None, OPTS.grow(), |layout| { + layout.vertical(None, OPTS.fill_xy(), |layout| { let hide_cursor = state.picker.is_some(); - screen::layout_screen(layout, size, state.screens.last().unwrap(), hide_cursor); + screen::layout_screen(layout, state.screens.last().unwrap(), hide_cursor); }); layout.vertical(None, OPTS, |layout| { @@ -52,8 +57,7 @@ pub(crate) fn ui(frame: &mut Frame, state: &mut State) { for item in layout.iter() { let LayoutItem { data, pos, size } = item; let area = Rect::new(pos[0], pos[1], size[0], size[1]); - let (text, style) = data; - frame.render_widget(SpanRef(text, *style), area); + frame.render_widget(data, area); } layout.clear(); @@ -61,12 +65,12 @@ pub(crate) fn ui(frame: &mut Frame, state: &mut State) { state.screens.last_mut().unwrap().size = frame.area().as_size(); } -struct SpanRef<'a>(&'a Cow<'a, str>, Style); - -impl<'a> Widget for SpanRef<'a> { +impl<'a> Widget for &UiItem<'a> { fn render(self, area: Rect, buf: &mut Buffer) { - let SpanRef(text, style) = self; - buf.set_string(area.x, area.y, text, style); + match self { + UiItem::Span(text, style) => buf.set_string(area.x, area.y, text, *style), + UiItem::Style(style) => buf.set_style(area, *style), + } } } @@ -127,8 +131,24 @@ pub(crate) fn layout_line<'a>(layout: &mut UiTree<'a>, line: Line<'a>) { } pub(crate) fn layout_span<'a>(layout: &mut UiTree<'a>, span: (Cow<'a, str>, Style)) { - let width = span.0.graphemes(true).count() as u16; - layout.leaf_with_size(span, [width, 1]); + match span.0 { + Cow::Borrowed(s) => { + for word in s.split_word_bounds() { + layout.leaf_with_size( + UiItem::Span(Cow::Borrowed(word), span.1), + [word.graphemes(true).count() as u16, 1], + ); + } + } + Cow::Owned(s) => { + for word in s.split_word_bounds() { + layout.leaf_with_size( + UiItem::Span(Cow::Owned(word.into()), span.1), + [word.graphemes(true).count() as u16, 1], + ); + } + } + } } pub(crate) fn repeat_chars(layout: &mut UiTree, count: usize, chars: &'static str, style: Style) { diff --git a/src/ui/layout/mod.rs b/src/ui/layout/mod.rs index 4438a9372f..9ccdfe0de9 100644 --- a/src/ui/layout/mod.rs +++ b/src/ui/layout/mod.rs @@ -81,6 +81,12 @@ impl TreeIndex { } } +#[derive(Debug, PartialEq, Copy, Clone)] +pub enum Pass { + Fit, + Fill, +} + impl LayoutTree { pub fn new() -> Self { LayoutTree { @@ -142,6 +148,7 @@ impl LayoutTree { ..opts }, size: Vec2(0, 0), + content: Vec2(0, 0), pos: None, }; @@ -164,6 +171,7 @@ impl LayoutTree { ..opts }, size: Vec2(0, 0), + content: Vec2(0, 0), pos: None, }; @@ -182,6 +190,7 @@ impl LayoutTree { data: Some(data), opts: OPTS, size: size.into(), + content: size.into(), pos: None, }); @@ -194,113 +203,174 @@ impl LayoutTree { }; let size = Vec2::from(avail_size); - self.compute_subtree(root, Vec2(0, 0), size, Vec2(0, 0), Sizing::Fit); + self.data[root].pos = Some(Vec2(0, 0)); - let grow = Vec2::from(avail_size).saturating_sub(self.data[root].size); - self.compute_subtree(root, Vec2(0, 0), avail_size.into(), grow, Sizing::Flex); + for pass in [Pass::Fit, Pass::Fill] { + if let Some(root_size) = self.compute_subtree(root, Vec2(0, 0), size, pass) { + self.data[root].size = root_size; + } + } } fn compute_subtree( &mut self, parent: usize, - start: Vec2, - avail_size: Vec2, - parent_grow: Vec2, - pass: Sizing, - ) { - let Some(child) = self.index.iter_children(parent).next() else { - return; - }; - - let Opts { - dir, - gap, - pad, - sizing, - .. - } = self.data[parent].opts; + outer_start: Vec2, + outer_avail_size: Vec2, + pass: Pass, + ) -> Option { + let child = self.index.iter_children(parent).next()?; + let parent_opts = self.data[parent].opts; + let main_axis = parent_opts.dir.axis(); + let cross_axis = main_axis.flip(); + let padding = Vec2(parent_opts.pad, parent_opts.pad) * main_axis; + let avail_size = outer_avail_size + .saturating_sub(padding) + .saturating_sub(padding); let mut current_child = Some(child); - let mut cursor = Vec2(pad, pad) * dir.axis(); + let mut cursor = Vec2(0, 0); let mut size = Vec2(0, 0); - let mut grow_iter = self.iter_distribute_size(parent, parent_grow, dir); + // Intrinsic extent, accumulated independently of any fill shrinking. + let mut content_cursor = Vec2(0, 0); + let mut content = Vec2(0, 0); - while let Some(child) = current_child { - let child_grow = grow_iter.next().unwrap(); - let child_avail_size = - if pass == Sizing::Flex && self.data[child].opts.sizing == Sizing::Flex { - self.data[child].size + child_grow - } else { - avail_size.saturating_sub(cursor) - }; + let fill_avail = match pass { + Pass::Fit => Vec2(0, 0), + Pass::Fill => avail_size.saturating_sub(self.data[parent].size), + }; - self.compute_subtree(child, start + cursor, child_avail_size, child_grow, pass); + let mut main_fill_iter = self.dist_main_fill(parent, fill_avail); + let cross_fill = avail_size * cross_axis; - let child_data = &mut self.data[child]; + while let Some(child) = current_child { + let child_start = outer_start + padding + cursor; + let is_main_fill = self.data[child].opts.is_main_fill(&parent_opts); + let is_cross_fill = self.data[child].opts.is_cross_fill(&parent_opts); + + let mut child_size = match pass { + Pass::Fit => { + let child_avail_size = avail_size.saturating_sub(cursor); + + // `flow` is the shrinkable extent, `child_content` the intrinsic one. + // They only differ where a descendant fills an axis. + let (flow, child_content) = + match self.compute_subtree(child, child_start, child_avail_size, pass) { + Some(flow) => (flow, self.data[child].content), + None => (self.data[child].size, self.data[child].size), + }; + + self.data[child].content = child_content; + content = content.max(content_cursor + child_content); + content_cursor += + main_axis * (Vec2(parent_opts.gap, parent_opts.gap) + child_content); + + // On an axis this child fills it may shrink, so take the flow + // extent. On an axis it does not fill, its size is its content. + let fill_mask = self.data[child].opts.fill; + let content_mask = Vec2(1, 1).saturating_sub(fill_mask); + let resolved = flow * fill_mask + child_content * content_mask; + + if is_main_fill { + // Zero-out the main axis that this child will later fill, + // this enables it to shrink. + resolved * cross_axis + } else { + resolved + } + } + Pass::Fill => { + let fill = { + let mut sum = Vec2(0, 0); + if is_main_fill { + sum += main_fill_iter.next().unwrap() + } + if is_cross_fill { + sum += cross_fill + } + sum + }; + + let child_avail_size = avail_size + .saturating_sub(cursor) + .min(self.data[child].size + fill); + + if self + .compute_subtree(child, child_start, child_avail_size, pass) + .is_some() + { + child_avail_size + } else { + self.data[child].size + } + } + }; - if (cursor + child_data.size).fits(avail_size) { - child_data.pos = Some(start + cursor); + let mut child_pos = None; + + if (cursor + child_size).fits(avail_size) { + child_pos = Some(outer_start + padding + cursor); } else { // Child doesn't fit where cursor currently is - // TODO Uncomment to include wrapping (tests below) - // // Try wrapping to next line/column first - // let next_line = size * dir.axis().flip(); - - // if (next_line + child_data.size).fits(avail_size) { - // // Fits completely on next line - // cursor = next_line; - // child_data.pos = Some(start + cursor); - // } else + let next_line = size * cross_axis; - if (cursor + Vec2(1, 1)).fits(avail_size) { + if (next_line + child_size).fits(avail_size) { + // Fits completely on next line + cursor = next_line; + child_pos = Some(outer_start + padding + cursor); + } else if (cursor + Vec2(1, 1)).fits(avail_size) { // Can't wrap, but we can fit at least one cell where the cursor currently is - child_data.pos = Some(start + cursor); - child_data.size = child_data.size.min(avail_size.saturating_sub(cursor)); - } else { - // There's absolutely no room left anywhere - child_data.pos = None; + child_pos = Some(outer_start + padding + cursor); + child_size = child_size.min(avail_size.saturating_sub(cursor)); } } - size = size.max(cursor + child_data.size); - cursor += dir.axis() * (Vec2(gap, gap) + child_data.size); + if child_pos.is_some() { + size = size.max(cursor + child_size); + cursor += main_axis * (Vec2(parent_opts.gap, parent_opts.gap) + child_size); + } + + self.data[child].pos = child_pos; + self.data[child].size = child_size; current_child = self.index.iter_siblings_after(child).next(); } - size += Vec2(pad, pad) * dir.axis(); + // Content is accumulated without wrapping, so it can come out smaller on + // the cross axis than what was actually laid out. Never report less than + // the placed extent. + let content = content.max(size); - if pass == Sizing::Flex && sizing == Sizing::Flex { - self.data[parent].size += parent_grow; - } else if pass == Sizing::Fit && sizing == Sizing::Flex { - // Zero-out the axis that is supposed to grow - self.data[parent].size = size * dir.axis().flip(); - } else { - self.data[parent].size = size; - } + size += padding + padding; + self.data[parent].content = content + padding + padding; + + Some(size) } - fn iter_distribute_size( + fn dist_main_fill( &self, parent: usize, size_to_distribute: Vec2, - dir: Direction, ) -> impl Iterator + use { - let along_axis = size_to_distribute * dir.axis(); - let div = if along_axis != Vec2(0, 0) { - self.index - .iter_children(parent) - .filter(|&child| self.data[child].opts.sizing == Sizing::Flex) - .count() - .max(1) as u16 + let axis = self.data[parent].opts.dir.axis(); + let along_axis = size_to_distribute * axis; + let count = self + .index + .iter_children(parent) + .filter(|&child| self.data[child].opts.is_main_fill(&self.data[parent].opts)) + .count() as u16; + + let (quot, rem) = if count > 0 { + let quot = along_axis / Vec2(count, count); + let rem = along_axis % Vec2(count, count); + (quot, rem) } else { - 1 + (Vec2(0, 0), Vec2(0, 0)) }; - let quot = along_axis / Vec2(div, div); - let rem = along_axis % Vec2(div, div); - let off_axis_amount = size_to_distribute * dir.axis().flip(); - iter::once(quot + rem + off_axis_amount).chain(iter::once(quot + off_axis_amount).cycle()) + iter::once(quot + rem) + .chain(iter::once(quot).cycle()) + .take(count as usize) } pub fn iter(&self) -> impl Iterator> { @@ -309,12 +379,17 @@ impl LayoutTree { data: Some(data), opts: _, size, + content: _, pos, } = &self.data[index] else { return None; }; + if size.0 == 0 || size.1 == 0 { + return None; + } + Some(LayoutItem { data, pos: (*pos)?.into(), @@ -365,6 +440,55 @@ mod tests { .join("\n") } + #[test] + fn test_iter_distribute_size_no_flex() { + let mut layout = LayoutTree::new(); + layout.vertical(None, OPTS, |layout| { + // Neither should grow + layout.text("Hello"); + layout.text("Hello"); + }); + + let mut iter = layout.dist_main_fill(0, Vec2(10, 3)); + assert_eq!(None, iter.next()); + } + + #[test] + fn test_iter_distribute_size_one_flex() { + let mut layout = LayoutTree::new(); + layout.vertical(None, OPTS, |layout| { + layout.horizontal(None, OPTS, |layout| { + layout.text("One"); + }); + // This should shrink to 0 vertically and then grow to 3 + layout.horizontal(None, OPTS.fill_y(), |layout| { + layout.text("Two"); + }); + }); + + let mut iter = layout.dist_main_fill(0, Vec2(10, 3)); + assert_eq!(Some(Vec2(0, 3)), iter.next()); + assert_eq!(None, iter.next()); + } + + #[test] + fn test_iter_distribute_size_all_flex() { + let mut layout = LayoutTree::new(); + layout.vertical(None, OPTS, |layout| { + // Both should grow, favoring the first + layout.horizontal(None, OPTS.fill_y(), |layout| { + layout.text("One"); + }); + layout.horizontal(None, OPTS.fill_y(), |layout| { + layout.text("Two"); + }); + }); + + let mut iter = layout.dist_main_fill(0, Vec2(10, 5)); + assert_eq!(Some(Vec2(0, 3)), iter.next()); + assert_eq!(Some(Vec2(0, 2)), iter.next()); + } + #[test] fn single_text() { let mut layout = LayoutTree::new(); @@ -457,42 +581,62 @@ mod tests { insta::assert_snapshot!(render_to_string(layout)); } - // TODO wrapping test (code commented above) - // #[test] - // fn test_horizontal_wrap() { - // let mut layout = LayoutTree::new(); - - // layout.horizontal(None, OPTS, |layout| { - // layout.text("AAA"); - // layout.text("BBB"); - // layout.text("CCC"); - // }); - - // layout.compute([6, 2]); - // let result = render_to_string(layout); - // println!("Result:\n{}", result); - // // Should wrap: "AAABBB" on first line, "CCC" on second line - // assert_eq!(result, "AAABBB\nCCC"); - // } - - // TODO wrapping test (code commented above) - // #[test] - // fn test_wrap_before_truncate() { - // let mut layout = LayoutTree::new(); - - // layout.horizontal(None, OPTS, |layout| { - // layout.text("AAAA"); - // layout.text("BBBB"); - // }); - - // layout.compute([6, 2]); - // let result = render_to_string(layout); - // println!("Result:\n{}", result); - // // With 6 chars width and 2 rows: - // // "AAAA" fits (4 chars), then "BBBB" doesn't fit in remaining 2 chars - // // Should wrap "BBBB" to next line rather than truncating to "BB" - // assert_eq!(result, "AAAA\nBBBB"); - // } + #[test] + fn test_horizontal_wrap() { + let mut layout = LayoutTree::new(); + + layout.horizontal(None, OPTS, |layout| { + layout.text("AAA"); + layout.text("BBB"); + layout.text("CCC"); + }); + + layout.compute([6, 2]); + let result = render_to_string(layout); + println!("Result:\n{}", result); + // Should wrap: "AAABBB" on first line, "CCC" on second line + assert_eq!(result, "AAABBB\nCCC"); + } + + #[test] + fn test_wrap_before_truncate() { + let mut layout = LayoutTree::new(); + + layout.horizontal(None, OPTS, |layout| { + layout.text("AAAA"); + layout.text("BBBB"); + }); + + layout.compute([6, 2]); + let result = render_to_string(layout); + println!("Result:\n{}", result); + // With 6 chars width and 2 rows: + // "AAAA" fits (4 chars), then "BBBB" doesn't fit in remaining 2 chars + // Should wrap "BBBB" to next line rather than truncating to "BB" + assert_eq!(result, "AAAA\nBBBB"); + } + + #[test] + fn nested_grow_wrap() { + let mut layout = LayoutTree::new(); + + layout.vertical(None, OPTS, |layout| { + layout.vertical(None, OPTS.fill_y(), |layout| { + layout.horizontal(None, OPTS, |layout| { + layout.text("word1"); + layout.text("word2"); + layout.text("word3"); + }); + }); + }); + + layout.compute([10, 2]); + let mut iter = layout.iter().map(|e| (*e.data, e.pos, e.size)); + assert_eq!(Some(("word1", [0, 0], [5, 1])), iter.next()); + assert_eq!(Some(("word2", [5, 0], [5, 1])), iter.next()); + assert_eq!(Some(("word3", [0, 1], [5, 1])), iter.next()); + assert_eq!(None, iter.next()); + } #[test] fn test_no_trailing_newline() { @@ -576,7 +720,7 @@ mod tests { let mut layout = LayoutTree::new(); layout.vertical(None, OPTS, |layout| { - layout.vertical(None, OPTS.grow(), |layout| { + layout.vertical(None, OPTS.fill_y(), |layout| { layout.text("flex"); }); layout.text("actual"); @@ -586,6 +730,26 @@ mod tests { insta::assert_snapshot!(render_to_string(layout)); } + #[test] + fn nested_grow_preserves_cross_axis() { + let mut layout = LayoutTree::new(); + + layout.vertical(Some("root"), OPTS, |layout| { + layout.horizontal(Some("grow"), OPTS.fill_xy(), |layout| { + layout.text("hello"); + }); + }); + + layout.compute([20, 10]); + + let mut iter = layout.iter().map(|e| (*e.data, e.pos, e.size)); + + assert_eq!(Some(("root", [0, 0], [20, 10])), iter.next()); + assert_eq!(Some(("grow", [0, 0], [20, 10])), iter.next()); + assert_eq!(Some(("hello", [0, 0], [5, 1])), iter.next()); + assert_eq!(None, iter.next()); + } + #[test] fn overflow() { let mut layout = LayoutTree::new(); @@ -593,9 +757,10 @@ mod tests { layout.vertical(None, OPTS, |layout| { layout.text("one"); layout.text("twoooo"); + layout.text("three"); }); - layout.compute([20, 1]); + layout.compute([6, 1]); insta::assert_snapshot!(render_to_string(layout)); } @@ -604,51 +769,97 @@ mod tests { let mut layout = LayoutTree::new(); layout.vertical(None, OPTS, |layout| { - layout.vertical(None, OPTS.grow(), |layout| { + layout.vertical(None, OPTS.fill_y(), |layout| { layout.text("flex 1"); layout.text("flex 2"); }); layout.text("actual"); }); - layout.compute([20, 2]); + layout.compute([6, 2]); insta::assert_snapshot!(render_to_string(layout)); } #[test] - fn gitu_mockup() { + fn shrinks_nested() { let mut layout = LayoutTree::new(); layout.vertical(None, OPTS, |layout| { - layout.vertical(None, OPTS.grow().gap(1), |layout| { + layout.vertical(None, OPTS.fill_y(), |layout| { layout.vertical(None, OPTS, |layout| { - layout.text("On branch master"); - layout.vertical(None, OPTS, |layout| { - layout.text("Your branch is up to date with 'origin/master'"); + layout.text("This should not be visible'"); + }); + }); + + layout.vertical(None, OPTS, |layout| { + layout.text("WEEEEE"); + }); + }); + + layout.compute([40, 1]); + let mut iter = layout.iter().map(|e| (*e.data, e.pos, e.size)); + assert_eq!(Some(("WEEEEE", [0, 0], [6, 1])), iter.next()); + assert_eq!(None, iter.next()); + } + + #[test] + fn nested_horizontal_fill_does_not_hide_siblings() { + let mut layout = LayoutTree::new(); + + layout.vertical(None, OPTS, |layout| { + // 0,0 -> 0,5 + layout.vertical(None, OPTS.fill_y(), |layout| { + // 0,1 -> 0,1 + layout.horizontal(None, OPTS.fill_x(), |layout| { + // 0,1 -> 0,1 + layout.horizontal(None, OPTS.fill_x(), |layout| { + layout.text("visible"); }); }); + }); + }); - layout.vertical(None, OPTS, |layout| { - layout.text("Recent commits"); - layout.vertical(None, OPTS, |layout| { - layout.text( - "9eb6a63 refactor/ui origin/refactor/ui fix more rendering issues", - ); - layout.text("b5fffd4 fix styling issues in Screen"); - layout.text("61e6c1b refactor: extract type of LayoutTree"); - layout.text("df3bcb5 get rid of frequent clone() in LayoutTree"); - layout.text("9864859 refactor(ui): less allocs"); - layout.text( - "aa2811e refactor: new LayoutTree module to improve on ui headaches", - ); - layout.text("5374ab3 master origin/master test: add file:// in clone_and_commit fn as well"); - layout.text("7a66235 test: get rid of setup_init, and try fix test-repo assertion"); - layout.text("75463c8 test/fix-ci test: forgot to create testfiles/ when running tests"); + layout.compute([20, 5]); + let mut iter = layout.iter().map(|e| (*e.data, e.pos, e.size)); + assert_eq!(Some(("visible", [0, 0], [7, 1])), iter.next()); + assert_eq!(None, iter.next()); + + // insta::assert_snapshot!(render_to_string(layout)); + } + + #[test] + fn gitu_mockup() { + let mut layout = LayoutTree::new(); + + layout.vertical(None, OPTS, |layout| { + layout.vertical(None, OPTS.fill_xy(), |layout| { + // Screen + layout.vertical(None, OPTS.fill_x(), |layout| { + layout.horizontal(Some(""), OPTS.fill_x(), |layout| { + layout.text("On branch master"); }); + layout.text("Your branch is up to date with 'origin/master'"); }); + + layout.text(""); + layout.text("Recent commits"); + layout.text("9eb6a63 refactor/ui origin/refactor/ui fix more rendering issues"); + layout.text("b5fffd4 fix styling issues in Screen"); + layout.text("61e6c1b refactor: extract type of LayoutTree"); + layout.text("df3bcb5 get rid of frequent clone() in LayoutTree"); + layout.text("9864859 refactor(ui): less allocs"); + layout.text("aa2811e refactor: new LayoutTree module to improve on ui headaches"); + layout.text( + "5374ab3 master origin/master test: add file:// in clone_and_commit fn as well", + ); + layout.text("7a66235 test: get rid of setup_init, and try fix test-repo assertion"); + layout.text( + "75463c8 test/fix-ci test: forgot to create testfiles/ when running tests", + ); }); layout.vertical(None, OPTS, |layout| { + // Menu layout.text("───────────────────────────────────────────────────────────────"); layout.horizontal(None, OPTS.gap(2), |layout| { @@ -705,17 +916,6 @@ mod tests { }); layout.compute([80, 25]); - let root = layout.index.iter_roots().next().unwrap(); - let root_size = layout.data[root].size; - eprintln!("Root size: {:?}", root_size); - let result = render_to_string(layout); - eprintln!("Result has {} lines", result.lines().count()); - eprintln!("Result ends with newline: {}", result.ends_with('\n')); - eprintln!("Result len: {}", result.len()); - eprintln!( - "Last 3 bytes: {:?}", - &result.as_bytes()[result.len().saturating_sub(3)..] - ); - insta::assert_snapshot!(result); + insta::assert_snapshot!(render_to_string(layout)); } } diff --git a/src/ui/layout/node.rs b/src/ui/layout/node.rs index 86e5779759..8c5f750453 100644 --- a/src/ui/layout/node.rs +++ b/src/ui/layout/node.rs @@ -2,26 +2,20 @@ use super::vec2::Vec2; use super::direction::Direction; -#[derive(Debug, PartialEq, Copy, Clone)] -pub enum Sizing { - Fit, - Flex, -} - pub const OPTS: Opts = Opts { dir: Direction::Horizontal, + fill: Vec2(0, 0), gap: 0, pad: 0, - sizing: Sizing::Fit, }; #[derive(Debug, Copy, Clone)] pub struct Opts { /// Layout direction for children of this node. pub(crate) dir: Direction, + pub(crate) fill: Vec2, /// The space between each direct child of this node. pub(crate) gap: u16, - pub(crate) sizing: Sizing, pub(crate) pad: u16, } @@ -32,20 +26,43 @@ impl Default for Opts { } impl Opts { - pub fn gap(self, gap: u16) -> Self { - Self { gap, ..self } + pub fn fill_x(self) -> Opts { + Self { + fill: Vec2(1, 0), + ..self + } } - pub fn grow(self) -> Opts { + #[allow(dead_code)] + pub fn fill_y(self) -> Opts { Self { - sizing: Sizing::Flex, + fill: Vec2(0, 1), ..self } } - pub(crate) fn pad(self, pad: u16) -> Opts { + pub fn fill_xy(self) -> Opts { + Self { + fill: Vec2(1, 1), + ..self + } + } + + pub fn gap(self, gap: u16) -> Self { + Self { gap, ..self } + } + + pub fn pad(self, pad: u16) -> Opts { Self { pad, ..self } } + + pub(crate) fn is_main_fill(&self, parent: &Opts) -> bool { + self.fill * parent.dir.axis() != Vec2(0, 0) + } + + pub(crate) fn is_cross_fill(&self, parent: &Opts) -> bool { + self.fill * parent.dir.axis().flip() != Vec2(0, 0) + } } #[derive(Debug)] @@ -55,7 +72,42 @@ pub(crate) struct Node { pub(crate) opts: Opts, /// space actually occupied by this node, updated as nodes are added pub(crate) size: Vec2, + /// Intrinsic (content) size of this subtree, ignoring fill shrinking. + /// A fill node may shrink below this, but an ancestor that does *not* fill + /// a given axis still needs the content extent to size itself on that axis. + pub(crate) content: Vec2, /// Offset from parent's top-left corner, updated as nodes are added. /// This will remain `None` if there's no valid position for the element. pub(crate) pos: Option, } + +#[cfg(test)] +mod tests { + + #[test] + fn is_axis_fill() { + let horizontal = super::Opts { + dir: super::Direction::Horizontal, + ..Default::default() + }; + let vertical = super::Opts { + dir: super::Direction::Vertical, + ..Default::default() + }; + + assert!(super::OPTS.fill_x().is_main_fill(&horizontal)); + assert!(super::OPTS.fill_y().is_main_fill(&vertical)); + assert!(super::OPTS.fill_xy().is_main_fill(&horizontal)); + assert!(super::OPTS.fill_xy().is_main_fill(&vertical)); + + assert!(!super::OPTS.fill_x().is_main_fill(&vertical)); + assert!(!super::OPTS.fill_y().is_main_fill(&horizontal)); + assert!(!super::OPTS.fill_x().is_cross_fill(&horizontal)); + assert!(!super::OPTS.fill_y().is_cross_fill(&vertical)); + + assert!(!super::OPTS.is_main_fill(&horizontal)); + assert!(!super::OPTS.is_main_fill(&vertical)); + assert!(!super::OPTS.is_cross_fill(&horizontal)); + assert!(!super::OPTS.is_cross_fill(&vertical)); + } +} diff --git a/src/ui/menu.rs b/src/ui/menu.rs index c3a7f2d091..a2cdf3a0a9 100644 --- a/src/ui/menu.rs +++ b/src/ui/menu.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use crate::ui::layout::OPTS; -use crate::ui::{self, UiTree}; +use crate::ui::{self, UiTree, repeat_chars}; use crate::{app::State, ops::Op, ui::layout_line}; use itertools::Itertools; use ratatui::{ @@ -52,7 +52,7 @@ pub(crate) fn layout_menu<'a>(layout: &mut UiTree<'a>, state: &'a State, width: }) .partition(|(op, _binds)| matches!(op, Op::OpenMenu(_))); - let line = item.to_line(Arc::clone(&config)); + let line = item.to_line(&config); let separator_style = Style::from(&style.separator); layout.vertical(None, OPTS, |layout| { @@ -170,17 +170,16 @@ pub(crate) fn layout_menu<'a>(layout: &mut UiTree<'a>, state: &'a State, width: } fn layout_keybinds_table<'a>(layout: &mut UiTree<'a>, items: Vec<(Line<'a>, Line<'a>)>) { - layout.horizontal(None, OPTS.gap(1), |layout| { - layout.vertical(None, OPTS, |layout| { - for (key, _) in items.iter() { - layout_line(layout, key.clone()); - } - }); + const SPACES: &str = " "; + let max_width = items.iter().fold(0, |acc, (x, _)| acc.max(x.width())) + 1; - layout.vertical(None, OPTS, |layout| { - for (_, value) in items { - layout_line(layout, value); - } - }); + layout.vertical(None, OPTS, |layout| { + for (key, value) in items.iter() { + layout.horizontal(None, OPTS, |layout| { + layout_line(layout, key.clone()); + repeat_chars(layout, max_width - key.width(), SPACES, Style::new()); + layout_line(layout, value.clone()); + }); + } }); } diff --git a/src/ui/picker.rs b/src/ui/picker.rs index 029e9db484..8ec3239dbd 100644 --- a/src/ui/picker.rs +++ b/src/ui/picker.rs @@ -176,7 +176,10 @@ mod tests { let x0 = item.pos[0] as usize; let y0 = item.pos[1] as usize; let item_width = item.size[0] as usize; - let text = &item.data.0; + let text = match item.data { + crate::ui::UiItem::Span(cow, _) => cow, + crate::ui::UiItem::Style(_) => "", + }; for (i, c) in text.chars().take(item_width).enumerate() { if y0 < height && x0 + i < width { From 7a8af92ec243335c32b5fdb4b28c3167eb2d15ef Mon Sep 17 00:00:00 2001 From: altsem Date: Sat, 25 Jul 2026 20:49:58 +0200 Subject: [PATCH 2/2] fix: calculate display width with unicode-width to handle emoji A lot of changed on this branch, so I'm porting your PR into here @cedricpinson closes: #539 --- Cargo.toml | 2 +- src/ui.rs | 5 +++-- src/ui/layout/mod.rs | 6 ++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 91d0b1b2ed..372224ce68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,6 @@ pretty_assertions = "1.4.1" temp-dir = "0.1.16" criterion = "0.8.1" insta = "1.46.0" -unicode-width = "0.2.0" temp-env = "0.3.6" stdext = "0.3.3" url = "2.5.7" @@ -77,4 +76,5 @@ tinyvec = "1.10.0" smashquote = "0.1.2" imara-diff = { version = "0.2.0", default-features = false } fuzzy-matcher = "0.3.7" +unicode-width = "0.2.0" url = "2.5.7" diff --git a/src/ui.rs b/src/ui.rs index b1cab4eb8f..9d8340c668 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -10,6 +10,7 @@ use ratatui::Frame; use ratatui::prelude::*; use tui_prompts::State as _; use unicode_segmentation::UnicodeSegmentation; +use unicode_width::UnicodeWidthStr; pub(crate) mod layout; mod menu; @@ -136,7 +137,7 @@ pub(crate) fn layout_span<'a>(layout: &mut UiTree<'a>, span: (Cow<'a, str>, Styl for word in s.split_word_bounds() { layout.leaf_with_size( UiItem::Span(Cow::Borrowed(word), span.1), - [word.graphemes(true).count() as u16, 1], + [UnicodeWidthStr::width(word) as u16, 1], ); } } @@ -144,7 +145,7 @@ pub(crate) fn layout_span<'a>(layout: &mut UiTree<'a>, span: (Cow<'a, str>, Styl for word in s.split_word_bounds() { layout.leaf_with_size( UiItem::Span(Cow::Owned(word.into()), span.1), - [word.graphemes(true).count() as u16, 1], + [UnicodeWidthStr::width(word) as u16, 1], ); } } diff --git a/src/ui/layout/mod.rs b/src/ui/layout/mod.rs index 9ccdfe0de9..8d5c8ee1d1 100644 --- a/src/ui/layout/mod.rs +++ b/src/ui/layout/mod.rs @@ -4,10 +4,9 @@ mod vec2; use std::iter; -use unicode_segmentation::UnicodeSegmentation; - use direction::Direction; use node::*; +use unicode_width::UnicodeWidthStr; use vec2::Vec2; pub use node::OPTS; @@ -127,8 +126,7 @@ impl LayoutTree<&'static str> { /// Add a text leaf, calculating size based on string length #[allow(dead_code)] pub fn text(&mut self, text: &'static str) -> &mut Self { - let width = text.graphemes(true).count(); - self.leaf_with_size(text, [width as u16, 1]); + self.leaf_with_size(text, [UnicodeWidthStr::width(text) as u16, 1]); self } }