fix(search): VSCode Search extension: bring back matched lines in search results. (#63524)

Due to changes in the code base, the search extension code when run from
`main` shows file names only in the search results page - no matches in
the files. This is a regression from the behavior in the deployed
extension.

Deployed extension:
<img width="1504" alt="Screenshot 2024-06-27 at 10 04 37"
src="https://github.com/sourcegraph/sourcegraph/assets/129280/edd97903-d03f-4612-98c8-c8f286f0cb3b">

Running from `main`:
<img width="1502" alt="Screenshot 2024-06-27 at 10 11 17"
src="https://github.com/sourcegraph/sourcegraph/assets/129280/d7aefcfe-3a25-4486-9fa6-a5e6bc7c6a8e">

Turns out the reason is because some shared code expects chunk matches,
while the search queries were all returning line matches.

Added support for line matches in the shared code, and then fixed an
issue with the search results display not keeping up with `MatchGroups`.

## Test plan
Build and run locally.

### Build
```
git switch peterguy/vscode-bring-back-matched-lines-in-search-results
cd client/vscode
pnpm run build
```
### Run
- Launch extension in VSCode: open the `Run and Debug` sidebar view in
VS Code, then select `Launch VS Code Extension` from the dropdown menu.
- Run a search using the search bar.
- See that the results contain matched lines in the files and not just a
list of file names. Compare to the currently-deployed extension - the
search results should look generally the same.
This commit is contained in:
Peter Guy 2024-06-27 12:24:51 -07:00 committed by GitHub
parent 0777ced17f
commit 72c00d397a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 51 additions and 78 deletions

View File

@ -37,6 +37,7 @@ describe('FileContentSearchResult', () => {
expect(getByTestId(container, 'result-container')).toBeVisible()
expect(getAllByTestId(container, 'result-container').length).toBe(1)
expect(getAllByTestId(container, 'file-search-result').length).toBe(1)
expect(getAllByTestId(container, 'file-match-children').length).toBe(1)
})
// TODO: add test that the collapse shows if there are too many results

View File

@ -21,6 +21,7 @@ import {
getFileMatchUrl,
getRepositoryUrl,
getRevision,
type LineMatch,
} from '@sourcegraph/shared/src/search/stream'
import { useSettings, type SettingsCascadeProps } from '@sourcegraph/shared/src/settings/settings'
import type { TelemetryV2Props } from '@sourcegraph/shared/src/telemetry'
@ -112,10 +113,7 @@ export const FileContentSearchResult: React.FunctionComponent<React.PropsWithChi
const contextLines = useMemo(() => settings?.['search.contextLines'] ?? 1, [settings])
const unhighlightedGroups: MatchGroup[] = useMemo(
() => reranker(result.chunkMatches?.map(chunkToMatchGroup) ?? []),
[result, reranker]
)
const unhighlightedGroups: MatchGroup[] = useMemo(() => reranker(matchesToMatchGroups(result)), [result, reranker])
const [expandedGroups, setExpandedGroups] = useState(unhighlightedGroups)
const collapsedGroups = truncateGroups(expandedGroups, 5, contextLines)
@ -299,6 +297,12 @@ function countHighlightRanges(groups: MatchGroup[]): number {
return groups.reduce((count, group) => count + group.matches.length, 0)
}
function matchesToMatchGroups(result: ContentMatch): MatchGroup[] {
return [
...(result.lineMatches?.map(lineToMatchGroup) ?? []),
...(result.chunkMatches?.map(chunkToMatchGroup) ?? []),
]
}
function chunkToMatchGroup(chunk: ChunkMatch): MatchGroup {
const matches = chunk.ranges.map(range => ({
startLine: range.start.line,
@ -315,3 +319,19 @@ function chunkToMatchGroup(chunk: ChunkMatch): MatchGroup {
endLine: chunk.contentStart.line + plaintextLines.length,
}
}
function lineToMatchGroup(line: LineMatch): MatchGroup {
const matches = line.offsetAndLengths.map(offsetAndLength => ({
startLine: line.lineNumber,
startCharacter: offsetAndLength[0],
endLine: line.lineNumber,
endCharacter: offsetAndLength[0] + offsetAndLength[1],
}))
return {
plaintextLines: [line.line],
highlightedHTMLRows: undefined, // populated lazily
matches,
startLine: line.lineNumber,
endLine: line.lineNumber + 1, // the matches support `endLine` == `startLine`, but MatchGroup requires `endLine` > `startLine`
}
}

View File

@ -3,14 +3,14 @@ import React, { type MouseEvent, type KeyboardEvent, useCallback } from 'react'
import classNames from 'classnames'
import type * as H from 'history'
import type { Observable } from 'rxjs'
import { map } from 'rxjs/operators'
import { CodeExcerpt } from '@sourcegraph/branded/src/search-ui/components'
import type { HoverMerged } from '@sourcegraph/client-api'
import type { Hoverifier } from '@sourcegraph/codeintellify'
import { SourcegraphURL } from '@sourcegraph/common'
import type { ActionItemAction } from '@sourcegraph/shared/src/actions/ActionItem'
import type { FetchFileParameters } from '@sourcegraph/shared/src/backend/file'
import type { MatchGroupMatch } from '@sourcegraph/shared/src/components/ranking/PerFileResultRanking'
import type { MatchGroup } from '@sourcegraph/shared/src/components/ranking/PerFileResultRanking'
import type { Controller as ExtensionsController } from '@sourcegraph/shared/src/extensions/controller'
import type { HoverContext } from '@sourcegraph/shared/src/hover/HoverOverlay.types'
import {
@ -24,31 +24,10 @@ import { SymbolKind } from '@sourcegraph/shared/src/symbols/SymbolKind'
import type { TelemetryProps } from '@sourcegraph/shared/src/telemetry/telemetryService'
import { Button, Code } from '@sourcegraph/wildcard'
import type { HighlightLineRange } from '../../../graphql-operations'
import { CodeExcerpt } from '../components/CodeExcerpt'
import { useOpenSearchResultsContext } from '../MatchHandlersContext'
import styles from './FileMatchChildren.module.scss'
export interface MatchGroup {
blobLines?: string[]
// The matches in this group to display.
matches: MatchGroupMatch[]
// The 1-based position of where the first match in the group.
position: {
line: number
character: number
}
// The 0-based start line of the group (inclusive.)
startLine: number
// The 0-based end line of the group (exclusive.)
endLine: number
}
interface FileMatchProps extends SettingsCascadeProps, TelemetryProps {
location?: H.Location
result: ContentMatch | SymbolMatch | PathMatch
@ -162,44 +141,29 @@ function navigateToFileOnMiddleMouseButtonClick(event: MouseEvent<HTMLElement>):
}
}
/**
* Convert a MatchGroup to a "position", which is just a tuple of line and character.
* Supports the "old" way of handling line-based matches without requiring a lot of redesign downstream.
* Limitation is that the `line` is taken from the first match in the group; other matches are ignored.
* @param group a MatchGroup, which could have multiple matches in it
* @returns a tuple of line and character
*/
function groupToPosition(group: MatchGroup): { line: number; character: number } {
return {
line: group.matches.length > 0 ? group.matches[0].startLine : group.startLine,
character: group.matches.length > 0 ? group.matches[0].startCharacter : 0,
}
}
export const FileMatchChildren: React.FunctionComponent<React.PropsWithChildren<FileMatchProps>> = props => {
const { result, grouped, fetchHighlightedFileLineRanges, telemetryService } = props
const { result, grouped } = props
console.log('FILE_MATCH', { result, grouped })
const { openFile, openSymbol } = useOpenSearchResultsContext()
const fetchHighlightedFileRangeLines = React.useCallback(
(startLine: number, endLine: number) => {
const startTime = Date.now()
return fetchHighlightedFileLineRanges(
{
repoName: result.repository,
commitID: result.commit || '',
filePath: result.path,
disableTimeout: false,
ranges: grouped.map(
(group): HighlightLineRange => ({
startLine: group.startLine,
endLine: group.endLine,
})
),
},
false
).pipe(
map(lines => {
telemetryService.log(
'search.latencies.frontend.code-load',
{ durationMs: Date.now() - startTime },
{ durationMs: Date.now() - startTime }
)
return lines[grouped.findIndex(group => group.startLine === startLine && group.endLine === endLine)]
})
)
},
[result, fetchHighlightedFileLineRanges, grouped, telemetryService]
)
const createCodeExcerptLink = (group: MatchGroup): string =>
SourcegraphURL.from(getFileMatchUrl(result)).setLineRange(group.position).toString()
SourcegraphURL.from(getFileMatchUrl(result)).setLineRange(groupToPosition(group)).toString()
/**
* This handler implements the logic to simulate the click/keyboard
@ -285,9 +249,7 @@ export const FileMatchChildren: React.FunctionComponent<React.PropsWithChildren<
<div>
{grouped.map((group, index) => (
<div
key={`linematch:${getFileMatchUrl(result)}${group.position.line}:${
group.position.character
}`}
key={`linematch:${getFileMatchUrl(result)}${group.startLine}:${group.endLine}`}
className={classNames('test-file-match-children-item-wrapper', styles.itemCodeWrapper)}
>
<div
@ -297,32 +259,22 @@ export const FileMatchChildren: React.FunctionComponent<React.PropsWithChildren<
styles.item,
styles.itemClickable
)}
onClick={event =>
navigateToFile(event, {
line: group.position.line,
character: group.position.character,
})
}
onClick={event => navigateToFile(event, groupToPosition(group))}
onMouseUp={navigateToFileOnMiddleMouseButtonClick}
onKeyDown={event =>
navigateToFile(event, {
line: group.position.line,
character: group.position.character,
})
}
onKeyDown={event => navigateToFile(event, groupToPosition(group))}
data-testid="file-match-children-item"
tabIndex={0}
role="link"
>
<CodeExcerpt
repoName={result.repository}
commitID={result.commit || ''}
commitID={result.commit ?? ''}
filePath={result.path}
startLine={group.startLine}
endLine={group.endLine}
highlightRanges={group.matches}
fetchHighlightedFileRangeLines={fetchHighlightedFileRangeLines}
blobLines={group.blobLines}
plaintextLines={group.plaintextLines}
highlightedLines={group.highlightedHTMLRows}
/>
</div>
</div>