Skip to content

fix(workspace): select terminal search matches at the original column - #805

Merged
Ishaan Gangwani (ishaan1124) merged 2 commits into
synthetic-sciences:mainfrom
aniruddhaadak80:fix/terminal-search-fold-columns
Sep 29, 2026
Merged

Ishaan Gangwani (ishaan1124) merged 2 commits into
synthetic-sciences:mainfrom
aniruddhaadak80:fix/terminal-search-fold-columns

Conversation

@aniruddhaadak80

@aniruddhaadak80 ANIRUDDHA ADAK (aniruddhaadak80) commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

What

Terminal scrollback search lowercases each line to compare, then hands the offsets it finds straight to xterm:

const value = line.toLocaleLowerCase()
return Array.from({ length: value.length }, (_, column) => column)
  .filter((column) => value.indexOf(needle, column) === column)
  .map((column) => ({ column, row, length: query.length }))

toLocaleLowerCase() can change a string's UTF-16 length — İ (U+0130, the only code point whose lowercase is longer under the default locale) lowercases to i + U+0307, one code unit becoming two — so an index into value is not an index into line. And length uses the un-folded query.length while the match spans needle.length folded code units.

The consumer selects against the original buffer line:

// terminal.tsx:308
t.select(match.column, match.row, match.length)

Why it matters

Measured:

line         "İST build"   length 9
lowercased   "i̇st build"  length 10

search "build" -> { column: 5, length: 5 }   true column in the original is 4
search "st"    -> { column: 2, length: 2 }   true column is 1
"abc build"    -> { column: 4 }             correct (ASCII)

So every match after an İ in the same line is highlighted one cell to the right. The row and the match count stay right, which is exactly why this reads as "search works, the highlight is just a bit off" rather than as a bug.

Verification

The three existing tests are all ASCII. The new ones fail before the fix:

(fail) terminal scrollback search > reports columns in the original line when folding changes its length
 4 pass  1 fail
(pass) finds every match with case-insensitive row and column coordinates
(pass) finds repeated and overlapping matches
(pass) returns no coordinates for an empty or missing query
(pass) reports columns in the original line when folding changes its length
(pass) spans a whole astral character instead of cutting it in half
(pass) still reports plain ASCII lines unchanged
 6 pass
 0 fail

The ASCII guard ("abc build" folds to itself) pins the ordinary path, and the three pre-existing tests are untouched.

The change

Each folded code unit carries its original span, and the match is translated back:

     .map((line, row) => {
-      const value = line.toLocaleLowerCase()
+      // Folding a character can change its length (U+0130 lowercases to two
+      // code units), so an offset in the folded line is not the cell xterm
+      // selects against the original buffer. Track the original span of each
+      // folded code unit and translate the match back. A start-only map with
+      // a +1 reads one short when a match ends in an astral character, whose
+      // single code point spans two units, so each unit carries its own end.
+      let value = ""
+      const start: number[] = []
+      const end: number[] = []
+      let offset = 0
+      for (const char of line) {
+        const folded = char.toLocaleLowerCase()
+        value += folded
+        for (let unit = 0; unit < folded.length; unit++) {
+          start.push(offset)
+          end.push(offset + char.length)
+        }
+        offset += char.length
+      }
       return Array.from({ length: value.length }, (_, column) => column)
         .filter((column) => value.indexOf(needle, column) === column)
-        .map((column) => ({ column, row, length: query.length }))
+        .map((column) => ({
+          column: start[column],
+          row,
+          length: end[column + needle.length - 1]! - start[column]!,
+        }))
     })

length is derived from the original line rather than from the query. indexOf guarantees column + needle.length <= value.length, so the trailing lookup is always in range.

Update addressing review: the first version tracked only a start offset and added one, which reads a match ending in an astral character one short — searching 😀 in x😀 gave length 1 and would highlight half the emoji. Each folded unit now carries its own end offset, and a regression test pins terminalMatches(["x😀"], "😀") to [{ column: 1, row: 0, length: 2 }]. Also corrected: U+0130 is the only code point that grows, and it does so under the default locale.

Frontend typecheck clean; touched files are Prettier-clean.

Fixes #804

@vercel

vercel Bot commented Sep 28, 2026

Copy link
Copy Markdown

ANIRUDDHA ADAK (@aniruddhaadak80) is attempting to deploy a commit to the InkVell Team on Vercel.

A member of the Team first needs to authorize it.

@ishaan1124 Ishaan Gangwani (ishaan1124) left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks. One regression to fix first, plus a correction to the description.

frontend/workspace/src/components/terminal-search.ts line 31 computes

length: origin[column + needle.length - 1]! + 1 - origin[column]!

That's one short when a match ends in an astral character. Searching 😀 in x😀 gives length 1 (main gives 2), so the highlight cuts the emoji in half. Tracking an end offset per folded unit alongside the start fixes it:

for (let unit = 0; unit < folded.length; unit++) { start.push(offset); end.push(offset + char.length) }
// ...
length: end[column + needle.length - 1]! - start[column]!

That passes your İ cases and gets the emoji lengths right. Please add expect(terminalMatches(["x😀"], "😀")).toEqual([{ column: 1, row: 0, length: 2 }]).

On the description: U+0130 is the only code point whose lowercase is longer, and it grows under the default locale, so Turkish locales aren't the ones most affected.

@aniruddhaadak80
ANIRUDDHA ADAK (aniruddhaadak80) force-pushed the fix/terminal-search-fold-columns branch 2 times, most recently from 4abef4e to 92aaaa3 Compare September 29, 2026 11:38
…ded one

The search folded each line to lower case and reported the offset it found,
which is an index into the folded line rather than the cell xterm selects
against the original buffer. Track the start and the end of every folded
code unit instead, and derive the length from the end of the last one:
taking it from the start was a code unit short whenever the match ended in
an astral character, so searching an emoji drew the highlight through half
of it.
@aniruddhaadak80
ANIRUDDHA ADAK (aniruddhaadak80) force-pushed the fix/terminal-search-fold-columns branch 2 times, most recently from 455ded3 to 9346b28 Compare September 29, 2026 13:20
@aniruddhaadak80

Copy link
Copy Markdown
Contributor Author

Done in 9346b28.

  • Each folded code unit now carries its own end offset alongside the start, and length is end[column + needle.length - 1] - start[column]. Your x😀 case reads length 2 rather than 1.
  • Added spans a whole astral character instead of cutting it in half, pinning terminalMatches(["x😀"], "😀") to [{ column: 1, row: 0, length: 2 }] and the match at the start of a line too. It fails against the previous start-only version.
  • The İ cases still pass, and the three pre-existing tests and the ASCII guard are untouched. indexOf guarantees the trailing lookup is in range.
  • The description is updated to match: it now says U+0130 is the only code point whose lowercase is longer and that it grows under the default locale, and the "Update addressing review" section describes the end-offset change and the new test. The test name in the body matches the one in the file.

Note the workspace suite is green apart from four tests that also fail on main in this environment (files pane > previews a Volume file, and a project bootstrap reconnect test that trips the 5 s happy-dom timeout) — nothing that touches terminal-search.

@aniruddhaadak80

Copy link
Copy Markdown
Contributor Author

All checks are green on 9346b286 — the rebase resolved the conflict and the new tests pass. Ishaan Gangwani (@ishaan1124) ready for another look whenever you have a moment.

@ishaan1124
Ishaan Gangwani (ishaan1124) dismissed their stale review September 29, 2026 15:33

The requested changes are in and verified; CI is green.

@ishaan1124
Ishaan Gangwani (ishaan1124) merged commit a4e3b36 into synthetic-sciences:main Sep 29, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terminal scrollback search highlights the wrong columns when a line contains a character that grows when lowercased

2 participants