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\"") + } + } + } +}