From 80b873d5151b9208c89e2cd4e7d0a193a5baea16 Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Tue, 14 Apr 2026 19:33:03 -0700 Subject: [PATCH] Fix for excessive arguments --- .../src/bottom_pane/list_selection_view.rs | 15 ++-- .../src/bottom_pane/selection_popup_common.rs | 81 +++++++++---------- 2 files changed, 44 insertions(+), 52 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index 8629c57208..c15b7123bc 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -23,6 +23,7 @@ use super::CancellationEvent; use super::bottom_pane_view::BottomPaneView; use super::popup_consts::MAX_POPUP_ROWS; use super::scroll_state::ScrollState; +use super::selection_popup_common::ColumnWidthConfig; pub(crate) use super::selection_popup_common::ColumnWidthMode; use super::selection_popup_common::GenericDisplayRow; use super::selection_popup_common::measure_rows_height_with_col_width_mode; @@ -91,10 +92,11 @@ pub(crate) fn side_by_side_layout_widths( /// One selectable item in the generic selection list. pub(crate) type SelectionAction = Box; +pub(crate) type SelectionToggleAction = Box; pub(crate) struct SelectionToggle { pub is_on: bool, - pub action: Box, + pub action: SelectionToggleAction, } /// Callback invoked whenever the highlighted item changes (arrow keys, search @@ -878,13 +880,13 @@ impl Renderable for ListSelectionView { // Measure wrapped height for up to MAX_POPUP_ROWS items. let rows = self.build_rows(); + let column_width = ColumnWidthConfig::new(self.col_width_mode, self.name_column_width); let rows_height = measure_rows_height_with_col_width_mode( &rows, &self.state, MAX_POPUP_ROWS, effective_rows_width.saturating_add(1), - self.col_width_mode, - self.name_column_width, + column_width, ); let header = self.active_header(); @@ -953,13 +955,13 @@ impl Renderable for ListSelectionView { let header_height = header.desired_height(inner_width); let tab_height = tab_bar_height(&self.tabs, self.active_tab_idx.unwrap_or(0), inner_width); let rows = self.build_rows(); + let column_width = ColumnWidthConfig::new(self.col_width_mode, self.name_column_width); let rows_height = measure_rows_height_with_col_width_mode( &rows, &self.state, MAX_POPUP_ROWS, effective_rows_width.saturating_add(1), - self.col_width_mode, - self.name_column_width, + column_width, ); // Stacked (fallback) side content height — only used when not side-by-side. @@ -1038,8 +1040,7 @@ impl Renderable for ListSelectionView { &self.state, render_area.height as usize, "no matches", - self.col_width_mode, - self.name_column_width, + column_width, ); } diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 810bfb528b..078bf9d619 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -55,6 +55,22 @@ pub(crate) enum ColumnWidthMode { Fixed, } +/// Column-width behavior plus an optional shared left-column width override. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct ColumnWidthConfig { + pub mode: ColumnWidthMode, + pub name_column_width: Option, +} + +impl ColumnWidthConfig { + pub(crate) const fn new(mode: ColumnWidthMode, name_column_width: Option) -> Self { + Self { + mode, + name_column_width, + } + } +} + // Fixed split used by explicitly fixed column mode: 30% label, 70% // description. const FIXED_LEFT_COLUMN_NUMERATOR: usize = 3; @@ -126,8 +142,7 @@ fn compute_desc_col( start_idx: usize, visible_items: usize, content_width: u16, - col_width_mode: ColumnWidthMode, - name_column_width: Option, + column_width: ColumnWidthConfig, ) -> usize { if content_width <= 1 { return 0; @@ -142,12 +157,12 @@ fn compute_desc_col( / FIXED_LEFT_COLUMN_DENOMINATOR) .max(1), ); - match col_width_mode { + match column_width.mode { ColumnWidthMode::Fixed => ((content_width as usize * FIXED_LEFT_COLUMN_NUMERATOR) / FIXED_LEFT_COLUMN_DENOMINATOR) .clamp(1, max_desc_col), ColumnWidthMode::AutoVisible | ColumnWidthMode::AutoAllRows => { - let max_name_width = match col_width_mode { + let max_name_width = match column_width.mode { ColumnWidthMode::AutoVisible => rows_all .iter() .enumerate() @@ -178,7 +193,8 @@ fn compute_desc_col( ColumnWidthMode::Fixed => 0, }; - name_column_width + column_width + .name_column_width .map(|width| width.max(max_name_width)) .unwrap_or(max_name_width) .saturating_add(2) @@ -378,8 +394,7 @@ fn adjust_start_for_wrapped_selection_visibility( desc_measure_items: usize, width: u16, viewport_height: u16, - col_width_mode: ColumnWidthMode, - name_column_width: Option, + column_width: ColumnWidthConfig, ) -> usize { let mut start_idx = compute_item_window_start(rows_all, state, max_items); let Some(sel) = state.selected_idx else { @@ -392,14 +407,8 @@ fn adjust_start_for_wrapped_selection_visibility( // If wrapped row heights push the selected item out of view, advance the // item window until the selected row is visible. while start_idx < sel { - let desc_col = compute_desc_col( - rows_all, - start_idx, - desc_measure_items, - width, - col_width_mode, - name_column_width, - ); + let desc_col = + compute_desc_col(rows_all, start_idx, desc_measure_items, width, column_width); if is_selected_visible_in_wrapped_viewport( rows_all, start_idx, @@ -513,8 +522,7 @@ fn render_rows_inner( state: &ScrollState, max_results: usize, empty_message: &str, - col_width_mode: ColumnWidthMode, - name_column_width: Option, + column_width: ColumnWidthConfig, ) -> u16 { if rows_all.is_empty() { if area.height > 0 { @@ -539,8 +547,7 @@ fn render_rows_inner( desc_measure_items, area.width, area.height, - col_width_mode, - name_column_width, + column_width, ); let desc_col = compute_desc_col( @@ -548,8 +555,7 @@ fn render_rows_inner( start_idx, desc_measure_items, area.width, - col_width_mode, - name_column_width, + column_width, ); // Render items, wrapping descriptions and aligning wrapped lines under the @@ -613,8 +619,7 @@ pub(crate) fn render_rows( state, max_results, empty_message, - ColumnWidthMode::AutoVisible, - None, + ColumnWidthConfig::default(), ) } @@ -631,8 +636,7 @@ pub(crate) fn render_rows_with_col_width_mode( state: &ScrollState, max_results: usize, empty_message: &str, - col_width_mode: ColumnWidthMode, - name_column_width: Option, + column_width: ColumnWidthConfig, ) -> u16 { render_rows_inner( area, @@ -641,8 +645,7 @@ pub(crate) fn render_rows_with_col_width_mode( state, max_results, empty_message, - col_width_mode, - name_column_width, + column_width, ) } @@ -688,8 +691,7 @@ pub(crate) fn render_rows_single_line( start_idx, visible_items, area.width, - ColumnWidthMode::AutoVisible, - None, + ColumnWidthConfig::default(), ); let mut cur_y = area.y; @@ -752,8 +754,7 @@ pub(crate) fn measure_rows_height( state, max_results, width, - ColumnWidthMode::AutoVisible, - None, + ColumnWidthConfig::default(), ) } @@ -765,17 +766,9 @@ pub(crate) fn measure_rows_height_with_col_width_mode( state: &ScrollState, max_results: usize, width: u16, - col_width_mode: ColumnWidthMode, - name_column_width: Option, + column_width: ColumnWidthConfig, ) -> u16 { - measure_rows_height_inner( - rows_all, - state, - max_results, - width, - col_width_mode, - name_column_width, - ) + measure_rows_height_inner(rows_all, state, max_results, width, column_width) } fn measure_rows_height_inner( @@ -783,8 +776,7 @@ fn measure_rows_height_inner( state: &ScrollState, max_results: usize, width: u16, - col_width_mode: ColumnWidthMode, - name_column_width: Option, + column_width: ColumnWidthConfig, ) -> u16 { if rows_all.is_empty() { return 1; // placeholder "no matches" line @@ -810,8 +802,7 @@ fn measure_rows_height_inner( start_idx, visible_items, content_width, - col_width_mode, - name_column_width, + column_width, ); let mut total: u16 = 0;