From 8f69209995298b495b6881b46ec7fbc38f0c1e77 Mon Sep 17 00:00:00 2001 From: Vidhaankhare16 Date: Tue, 15 Sep 2026 16:18:06 +0530 Subject: [PATCH] fix(search): shift Spotlight file-name highlights by the platform separator searchFiles matches the query against IndexedFile.relativePath, then rebases the resulting ranges onto the file name, because FileResult's matchRanges index the name. It found the name's start with lastIndexOf('/'), but relativePath is derived from two absolute paths and so is joined with a backslash on Windows. The search returned -1, the ranges were shifted by nothing, and the dialog underlined whatever characters happened to sit at those offsets: `uiButton` against `app\ui\Button.kt` underlined "on" and "kt" rather than "Button", and a match landing past the name's length was dropped and underlined nothing. Look for the separator alongside '/', not instead of it. On a POSIX host the two are the same character, so that host is unchanged; on Windows '/' is not a legal file-name character and cannot split a name by accident. A bare lastIndexOf('\') on every platform would be wrong, since a backslash is a legal POSIX file-name character. ContentSearchService in this package already reaches for File.separatorChar for this reason. The separator is a parameter defaulting to the host's. Because the rule is deliberately platform-dependent, a Windows-shaped path proves nothing on a POSIX host - the shift there is correctly zero - so injecting it is what lets the Windows case be asserted on every runner rather than only on a Windows one. The four shift cases do that; the end-to-end case uses the host separator and is red only on Windows, which its KDoc states. --- .../rever/boss/search/GlobalSearchService.kt | 28 ++++- .../search/GlobalSearchFileHighlightTest.kt | 115 ++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 composeApp/src/desktopTest/kotlin/ai/rever/boss/search/GlobalSearchFileHighlightTest.kt diff --git a/composeApp/src/commonMain/kotlin/ai/rever/boss/search/GlobalSearchService.kt b/composeApp/src/commonMain/kotlin/ai/rever/boss/search/GlobalSearchService.kt index 33b9936a0e..39ebdc4650 100644 --- a/composeApp/src/commonMain/kotlin/ai/rever/boss/search/GlobalSearchService.kt +++ b/composeApp/src/commonMain/kotlin/ai/rever/boss/search/GlobalSearchService.kt @@ -17,6 +17,7 @@ import kotlinx.coroutines.async import kotlinx.coroutines.awaitAll import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.withContext +import java.io.File private val logger = BossLogger.forComponent("GlobalSearchService") @@ -239,6 +240,31 @@ object GlobalSearchService { } } + /** + * Index in [relativePath] where the file name starts, for either platform's separator. + * + * [IndexedFile.relativePath] is produced by relativizing two absolute paths, so on Windows it + * arrives joined with `\` and a bare `lastIndexOf('/')` returns -1. The match ranges below are + * then rebased against 0 instead of the file name's first character, which underlines the wrong + * characters - or, once a range lands past the name's length, underlines nothing at all. + * + * `/` is checked ALONGSIDE [File.separatorChar] rather than instead of it. On a POSIX host the + * two are the same character, so that host's behaviour is unchanged; on Windows `/` is not a + * legal file-name character, so it cannot split a name by accident. Deliberately not a bare + * `lastIndexOf('\\')` on every platform: a backslash IS a legal POSIX file-name character, and + * that would split `dir/we\ird.kt` inside its own name. [ContentSearchService] reaches for + * [File.separatorChar] in this package for the same reason. + * + * [separator] defaults to the host's and is only passed explicitly by tests. Because the rule + * is deliberately platform-dependent, a Windows-shaped path proves nothing when fed to a POSIX + * host - the shift there is correctly zero - so the parameter is what lets the Windows case be + * asserted on every runner rather than only on a Windows one. + */ + internal fun fileNameStartIn( + relativePath: String, + separator: Char = File.separatorChar, + ): Int = maxOf(relativePath.lastIndexOf(separator), relativePath.lastIndexOf('/')) + 1 + /** * Search files using fuzzy matching. */ @@ -270,7 +296,7 @@ object GlobalSearchService { val pathMatch = FuzzyMatcher.match(query, file.relativePath, file.relativePath.lowercase()) if (pathMatch != null && pathMatch.score >= MIN_SCORE) { - val fileNameStart = file.relativePath.lastIndexOf('/') + 1 + val fileNameStart = fileNameStartIn(file.relativePath) val adjustedRanges = pathMatch.matchRanges .filter { it.start >= fileNameStart || it.end > fileNameStart } diff --git a/composeApp/src/desktopTest/kotlin/ai/rever/boss/search/GlobalSearchFileHighlightTest.kt b/composeApp/src/desktopTest/kotlin/ai/rever/boss/search/GlobalSearchFileHighlightTest.kt new file mode 100644 index 0000000000..2739f9e17d --- /dev/null +++ b/composeApp/src/desktopTest/kotlin/ai/rever/boss/search/GlobalSearchFileHighlightTest.kt @@ -0,0 +1,115 @@ +package ai.rever.boss.search + +import kotlinx.coroutines.runBlocking +import java.io.File +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Pins where a path-only file hit draws its underline. + * + * [SearchResult.FileResult.matchRanges] index the file NAME, while the match that produced them was + * found in the whole relative path, so the ranges have to be shifted back past the leading + * directories. That shift used to look for `/` only. [IndexedFile.relativePath] is built by + * relativizing two absolute paths, so on Windows it is joined with `\` and the search found no + * separator at all - the ranges were then shifted by nothing and underlined whatever characters + * happened to sit at those offsets in the name. + * + * The rule is deliberately platform-dependent, so the Windows case cannot be proven by feeding a + * backslash path to a POSIX host - the shift there is correctly zero. The separator is injected + * instead, which is what makes the two shift tests below assert the same thing on every runner. + * The end-to-end test uses the HOST's separator, and so is a red test only on Windows; it is a + * regression guard elsewhere, and is written that way on purpose rather than by accident. + */ +class GlobalSearchFileHighlightTest { + @BeforeTest + fun setUp() { + SearchSources.clearForTests() + } + + @AfterTest + fun tearDown() { + SearchSources.clearForTests() + } + + private companion object { + /** Built from its code point so the separator survives any tooling that rewrites escapes. */ + val BACKSLASH: Char = Char(92) + + /** `uiButton` is a subsequence of `appuiButton.kt` but NOT of `Button.kt`. */ + const val QUERY = "uiButton" + + const val NAME = "Button.kt" + } + + // ---- the shift itself, asserted identically on every OS ---- + + @Test + fun `a windows path shifts past its last backslash`() { + val relative = "app${BACKSLASH}ui${BACKSLASH}Button.kt" + + assertEquals(7, GlobalSearchService.fileNameStartIn(relative, separator = BACKSLASH)) + } + + @Test + fun `a posix path shifts past its last slash on any host`() { + assertEquals(7, GlobalSearchService.fileNameStartIn("app/ui/Button.kt", separator = '/')) + } + + @Test + fun `a bare file name shifts by nothing`() { + assertEquals(0, GlobalSearchService.fileNameStartIn(NAME, separator = BACKSLASH)) + assertEquals(0, GlobalSearchService.fileNameStartIn(NAME, separator = '/')) + } + + @Test + fun `a backslash inside a posix file name is not mistaken for a separator`() { + // A backslash is a legal POSIX file-name character. With '/' as the separator the name + // starts after the slash, not after the backslash inside it. + val relative = "dir/we${BACKSLASH}ird.kt" + + assertEquals(4, GlobalSearchService.fileNameStartIn(relative, separator = '/')) + } + + // ---- what the dialog actually draws, through the real search ---- + + /** + * Red on Windows, where the separator is `\`. On a POSIX host the shift already worked, so this + * guards against a regression rather than proving the fix. + */ + @Test + fun `a path hit underlines the file name, not an offset slice of it`() { + val sep = File.separatorChar + val file = + IndexedFile( + name = NAME, + path = "${File.separator}proj${sep}app${sep}ui$sep$NAME", + relativePath = "app${sep}ui$sep$NAME", + ) + + val result = + runBlocking { + GlobalSearchService + .search(rawQuery = QUERY, windowId = null, indexedFiles = listOf(file)) + .filterIsInstance() + .single() + } + + val underlined = result.matchRanges.map { result.name.substring(it.start, it.end) } + + // Before the fix, on Windows, this was ["on", "kt"]: the ranges for `ui` and `Button` in the + // full path were rebased against 0, landing on "Butt(on)" and "Button.(kt)". + assertEquals(listOf("Button"), underlined, "underlined text") + assertEquals(listOf(MatchRange(0, 6)), result.matchRanges, "ranges into the file name") + + val wanted = QUERY.lowercase().toSet() + for (text in underlined) { + for (ch in text.lowercase()) { + assertTrue(ch in wanted, "underlined '$ch' from \"$text\", which is not in \"$QUERY\"") + } + } + } +}