Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .jules/bolt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 2023-10-27 - Terminal Search Hot Loop Case Folding

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the one-off .jules artifact

Remove this automation note or keep any lasting rationale beside the owning search code/tests. As added, .jules/ is a new top-level directory containing only a task-specific learning record, with no build or contributor workflow consuming it, so it violates the repository contract that new root categories be durable and feature-specific material stay near its owner.

AGENTS.md reference: AGENTS.md:L131-L131

Useful? React with 👍 / 👎.

**Learning:** In the `terminal_search.rs` file, the `chars_eq_ignore_case` fallback invokes two `.to_lowercase()` iterators which is too slow for the hot path of searching a large scrollback. We can optimize it by short-circuiting ASCII characters using pre-calculated lowercase and uppercase bounds. However, a surprising edge case is that some non-ASCII characters (like the Kelvin sign `\u{212A}`) map to an ASCII character (`k`) when lowercased. Therefore, we must NOT reject a haystack character solely based on `h != first_lower && h != first_upper` if it's not pure ASCII. We must fallback to `chars_eq_ignore_case` for any non-ASCII haystack characters to preserve correctness.
**Action:** When implementing ASCII fast-paths for string or character comparisons in Rust, ensure non-ASCII inputs properly fallback to full unicode case folding routines, even if the search query (needle) is purely ASCII, to avoid dropping matches for homoglyphs like Kelvin sign.
69 changes: 52 additions & 17 deletions crates/forktty-ui-gtk/src/gtk_app/terminal_search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,25 +75,60 @@ fn for_each_char_match_start(
return;
}
let first_needle = needle[0];
let mut index = 0;
while index + needle.len() <= haystack.len() {
// Fast-path: short-circuit the full substring check if the first character
// doesn't match, avoiding iterator overhead in the common case.
if !chars_eq_ignore_case(haystack[index], first_needle) {
index += 1;
continue;

// Fast-path for the most common case: ASCII search queries.
if first_needle.is_ascii() {
Comment on lines +79 to +80

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Record the terminal-search speedup in the changelog

Add an Unreleased changelog entry for this optimization. For users searching large scrollbacks, the new fast path intentionally changes visible search responsiveness—the commit reports roughly halving a missed-query scan—but the patch leaves CHANGELOG.md unchanged, so this release-facing improvement will not be documented.

AGENTS.md reference: AGENTS.md:L183-L183

Useful? React with 👍 / 👎.

let first_lower = first_needle.to_ascii_lowercase();
let first_upper = first_needle.to_ascii_uppercase();

let mut index = 0;
while index + needle.len() <= haystack.len() {
let h = haystack[index];

// Check direct ASCII match first to avoid function call overhead
if h != first_lower && h != first_upper {
// If it's pure ASCII and doesn't match, we can safely skip.
// For non-ASCII haystack characters, we must fallback to the full
// unicode-aware check, as characters like Kelvin sign ('\u{212A}')
// uppercase/lowercase to ASCII 'k'.
if h.is_ascii() || !chars_eq_ignore_case(h, first_needle) {
index += 1;
continue;
}
}

let matched = haystack[index + 1..index + needle.len()]
.iter()
.zip(&needle[1..])
.all(|(a, b)| chars_eq_ignore_case(*a, *b));
if matched {
if !visit(index) {
return;
}
index += needle.len();
} else {
index += 1;
}
}
let matched = haystack[index + 1..index + needle.len()]
.iter()
.zip(&needle[1..])
.all(|(a, b)| chars_eq_ignore_case(*a, *b));
if matched {
if !visit(index) {
return;
} else {
let mut index = 0;
while index + needle.len() <= haystack.len() {
if !chars_eq_ignore_case(haystack[index], first_needle) {
index += 1;
continue;
}
let matched = haystack[index + 1..index + needle.len()]
.iter()
.zip(&needle[1..])
.all(|(a, b)| chars_eq_ignore_case(*a, *b));
if matched {
if !visit(index) {
return;
}
index += needle.len();
} else {
index += 1;
}
index += needle.len();
} else {
index += 1;
}
}
}
Expand Down
Loading