From 39adbb984d72fb4f93b62546ec0f8385a41110fe Mon Sep 17 00:00:00 2001 From: Wez Furlong Date: Mon, 25 Jul 2022 18:31:27 -0700 Subject: [PATCH] Pane::search: expose range, limit. Limit quickselect by default The recent work on the scrollback made it easier to constrain the search region, so expose those parameters to the Pane::search interface and to the mux protocol. Use those new parameters to constrain quickselect search to 1000 rows above and below the current viewport by default, and add a new parameter to QuickSelectArgs that allows overriding that range. A follow-up commit could make the search/copy overlay issue a series of searches in chunks so that it avoids blocking the UI when searching very large scrollback. refs: https://github.com/wez/wezterm/pull/1317 --- codec/src/lib.rs | 4 ++- config/src/keyassignment.rs | 3 +++ docs/changelog.md | 1 + .../lua/keyassignment/QuickSelectArgs.md | 1 + mux/src/localpane.rs | 26 ++++++++++++++----- mux/src/pane.rs | 18 ++++++++++--- mux/src/renderable.rs | 2 ++ term/src/screen.rs | 8 ++++-- wezterm-client/src/pane/clientpane.rs | 9 ++++++- wezterm-gui/src/overlay/copy.rs | 8 ++++-- wezterm-gui/src/overlay/quickselect.rs | 10 ++++++- wezterm-mux-server-impl/src/sessionhandler.rs | 18 ++++++++++--- 12 files changed, 88 insertions(+), 20 deletions(-) diff --git a/codec/src/lib.rs b/codec/src/lib.rs index e1892856c..1ac13b6d8 100644 --- a/codec/src/lib.rs +++ b/codec/src/lib.rs @@ -416,7 +416,7 @@ macro_rules! pdu { /// The overall version of the codec. /// This must be bumped when backwards incompatible changes /// are made to the types and protocol. -pub const CODEC_VERSION: usize = 26; +pub const CODEC_VERSION: usize = 27; // Defines the Pdu enum. // Each struct has an explicit identifying number. @@ -981,6 +981,8 @@ pub struct GetLinesResponse { pub struct SearchScrollbackRequest { pub pane_id: PaneId, pub pattern: mux::pane::Pattern, + pub range: Range, + pub limit: Option, } #[derive(Deserialize, Serialize, PartialEq, Debug)] diff --git a/config/src/keyassignment.rs b/config/src/keyassignment.rs index ff792d84c..75f6e6943 100644 --- a/config/src/keyassignment.rs +++ b/config/src/keyassignment.rs @@ -330,6 +330,9 @@ pub struct QuickSelectArguments { /// Label to use in place of "copy" when `action` is set #[dynamic(default)] pub label: String, + /// How man lines before and how many lines after the viewport to + /// search to produce the quickselect results + pub scope_lines: Option, } #[derive(Debug, Clone, PartialEq, Eq, FromDynamic, ToDynamic)] diff --git a/docs/changelog.md b/docs/changelog.md index c063f3e0c..e8f3986fb 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -46,6 +46,7 @@ As features stabilize some brief notes about them will accumulate here. [#2253](https://github.com/wez/wezterm/issues/2253) * Internal scrollback datastructure improvements reduce per-cell overhead by up to ~40x depending on the composition of the line (lines with lots of varied attributes or image attachments will have more overhead). * Improved search performance +* Quickselect: now defaults to searching 1000 lines above and below the current viewport, making it faster and the labels shorter for users with a larger scrollback. A new `scope_lines` parmeter to [QuickSelectArgs](config/lua/keyassignment/QuickSelectArgs.md) allows controlling the search region explicitly. Thanks to [@yyogo](https://github.com/yyogo) for the initial PR! [#1317](https://github.com/wez/wezterm/pull/1317) #### Fixed * [ActivateKeyTable](config/lua/keyassignment/ActivateKeyTable.md)'s `replace_current` field was not actually optional. Made it optional. [#2179](https://github.com/wez/wezterm/issues/2179) diff --git a/docs/config/lua/keyassignment/QuickSelectArgs.md b/docs/config/lua/keyassignment/QuickSelectArgs.md index 5fae5469b..f8b995c84 100644 --- a/docs/config/lua/keyassignment/QuickSelectArgs.md +++ b/docs/config/lua/keyassignment/QuickSelectArgs.md @@ -34,6 +34,7 @@ The `QuickSelectArgs` struct allows for the following fields: * `alphabet` - if present, this alphabet is used instead of [quick_select_alphabet](../config/quick_select_alphabet.md) * `action` - if present, this key assignment action is performed as if by [window:perform_action](../window/perform_action.md) when an item is selected. The normal clipboard action is NOT performed in this case. * `label` - if present, replaces the string `"copy"` that is shown at the bottom of the overlay; you can use this to indicate which action will happen if you are using `action`. +* `scope_lines` - Specify the number of lines to search above and below the current viewport. The default is 1000 lines. The scope will be increased to the current viewport height if it is smaller than the viewport. (*Since: nightly builds only*. In earlier releases, the entire scrollback was always searched). Here's an example that shows how to trigger some lua code to operate on the quick-selected text, instead of copying it to the clipboard. Here, we open diff --git a/mux/src/localpane.rs b/mux/src/localpane.rs index 165f2f5c2..ce6a98c21 100644 --- a/mux/src/localpane.rs +++ b/mux/src/localpane.rs @@ -476,7 +476,12 @@ impl Pane for LocalPane { term.get_semantic_zones() } - async fn search(&self, pattern: Pattern) -> anyhow::Result> { + async fn search( + &self, + pattern: Pattern, + range: Range, + limit: Option, + ) -> anyhow::Result> { let term = self.terminal.borrow(); let screen = term.screen(); @@ -498,11 +503,17 @@ impl Pane for LocalPane { let mut results = vec![]; let mut uniq_matches: HashMap = HashMap::new(); - let first_row = screen.phys_to_stable_row_index(0); - let all_stable = first_row..first_row + screen.scrollback_rows() as StableRowIndex; - screen.for_each_logical_line_in_stable_range(all_stable, |sr, lines| { + screen.for_each_logical_line_in_stable_range(range, |sr, lines| { + if let Some(limit) = limit { + if results.len() == limit as usize { + // We've reach the limit, stop iteration. + return false; + } + } + if lines.is_empty() { - return; + // Nothing to do on this iteration, carry on with the next. + return true; } let haystack = if lines.len() == 1 { lines[0].as_str() @@ -516,7 +527,7 @@ impl Pane for LocalPane { let stable_idx = sr.start; if haystack.is_empty() { - return; + return true; } let haystack = match &pattern { @@ -563,6 +574,9 @@ impl Pane for LocalPane { } } } + + // Keep iterating + true }); #[derive(Copy, Clone, Debug)] diff --git a/mux/src/pane.rs b/mux/src/pane.rs index 0aceb786d..0f22e8904 100644 --- a/mux/src/pane.rs +++ b/mux/src/pane.rs @@ -394,10 +394,22 @@ pub trait Pane: Downcast { false } - /// Performs a search. + /// Performs a search bounded to the specified range. /// If the result is empty then there are no matches. - /// Otherwise, the result shall contain all possible matches. - async fn search(&self, _pattern: Pattern) -> anyhow::Result> { + /// Otherwise, if limit.is_none(), the result shall contain all possible + /// matches. + /// If limit.is_some(), then the maximum number of results that will be + /// returned is limited to the specified number, and the + /// SearchResult::start_y of the last item + /// in the result can be used as the start of the next region to search. + /// You can tell that you have reached the end of the results if the number + /// of results is smaller than the limit you set. + async fn search( + &self, + _pattern: Pattern, + _range: Range, + _limit: Option, + ) -> anyhow::Result> { Ok(vec![]) } diff --git a/mux/src/renderable.rs b/mux/src/renderable.rs index 385a4375f..8b8f91061 100644 --- a/mux/src/renderable.rs +++ b/mux/src/renderable.rs @@ -100,6 +100,8 @@ pub fn terminal_get_logical_lines( logical, first_row: sr.start, }); + + true }); result } diff --git a/term/src/screen.rs b/term/src/screen.rs index f7e513521..4d674174a 100644 --- a/term/src/screen.rs +++ b/term/src/screen.rs @@ -927,7 +927,7 @@ impl Screen { stable_range: Range, mut f: F, ) where - F: FnMut(Range, &[&Line]), + F: FnMut(Range, &[&Line]) -> bool, { let mut phys_range = self.stable_range(&stable_range); @@ -990,7 +990,11 @@ impl Screen { break; } - f(logical_stable_range, &line_vec); + let continue_iteration = f(logical_stable_range, &line_vec); + + if !continue_iteration { + break; + } } } } diff --git a/wezterm-client/src/pane/clientpane.rs b/wezterm-client/src/pane/clientpane.rs index d1677eb5d..af8efb233 100644 --- a/wezterm-client/src/pane/clientpane.rs +++ b/wezterm-client/src/pane/clientpane.rs @@ -313,13 +313,20 @@ impl Pane for ClientPane { Ok(()) } - async fn search(&self, pattern: Pattern) -> anyhow::Result> { + async fn search( + &self, + pattern: Pattern, + range: Range, + limit: Option, + ) -> anyhow::Result> { match self .client .client .search_scrollback(SearchScrollbackRequest { pane_id: self.remote_pane_id, pattern, + range, + limit, }) .await { diff --git a/wezterm-gui/src/overlay/copy.rs b/wezterm-gui/src/overlay/copy.rs index 55906d086..9ab969236 100644 --- a/wezterm-gui/src/overlay/copy.rs +++ b/wezterm-gui/src/overlay/copy.rs @@ -274,8 +274,12 @@ impl CopyRenderable { let window = self.window.clone(); let pattern = self.pattern.clone(); promise::spawn::spawn(async move { - log::debug!("Searching for {pattern:?}"); - let mut results = pane.search(pattern).await?; + let dims = pane.get_dimensions(); + let range = dims.scrollback_top + ..dims.scrollback_top + dims.scrollback_rows as StableRowIndex; + let limit = None; + log::debug!("Searching for {pattern:?} in {range:?}"); + let mut results = pane.search(pattern, range, limit).await?; log::debug!("Sorting {} results", results.len()); results.sort(); log::debug!("Sorted"); diff --git a/wezterm-gui/src/overlay/quickselect.rs b/wezterm-gui/src/overlay/quickselect.rs index ffc724fc2..635520b8d 100644 --- a/wezterm-gui/src/overlay/quickselect.rs +++ b/wezterm-gui/src/overlay/quickselect.rs @@ -651,8 +651,16 @@ impl QuickSelectRenderable { let pane: Rc = self.delegate.clone(); let window = self.window.clone(); let pattern = self.pattern.clone(); + let scope = self.args.scope_lines; + let viewport = self.viewport; promise::spawn::spawn(async move { - let mut results = pane.search(pattern).await?; + let dims = pane.get_dimensions(); + let scope = scope.unwrap_or(1000).max(dims.viewport_rows); + let top = viewport.unwrap_or(dims.physical_top); + let range = top.saturating_sub(scope as StableRowIndex) + ..top + (dims.viewport_rows + scope) as StableRowIndex; + let limit = None; + let mut results = pane.search(pattern, range, limit).await?; results.sort(); let pane_id = pane.pane_id(); diff --git a/wezterm-mux-server-impl/src/sessionhandler.rs b/wezterm-mux-server-impl/src/sessionhandler.rs index a87e1fa4c..4ce5b3f0f 100644 --- a/wezterm-mux-server-impl/src/sessionhandler.rs +++ b/wezterm-mux-server-impl/src/sessionhandler.rs @@ -401,23 +401,33 @@ impl SessionHandler { .detach(); } - Pdu::SearchScrollbackRequest(SearchScrollbackRequest { pane_id, pattern }) => { + Pdu::SearchScrollbackRequest(SearchScrollbackRequest { + pane_id, + pattern, + range, + limit, + }) => { use mux::pane::Pattern; - async fn do_search(pane_id: TabId, pattern: Pattern) -> anyhow::Result { + async fn do_search( + pane_id: TabId, + pattern: Pattern, + range: std::ops::Range, + limit: Option, + ) -> anyhow::Result { let mux = Mux::get().unwrap(); let pane = mux .get_pane(pane_id) .ok_or_else(|| anyhow!("no such pane {}", pane_id))?; - pane.search(pattern).await.map(|results| { + pane.search(pattern, range, limit).await.map(|results| { Pdu::SearchScrollbackResponse(SearchScrollbackResponse { results }) }) } spawn_into_main_thread(async move { promise::spawn::spawn(async move { - let result = do_search(pane_id, pattern).await; + let result = do_search(pane_id, pattern, range, limit).await; send_response(result); }) .detach();