Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 7dbc44a

Browse files
fklingvovakulikov
authored andcommitted
code intel: Remove search context from search based code intel (#58010)
Using search context is problematic when viewing a file/repo that is not part of the selected search context, which can happen when a custom default context is set. It doesn't seem necessary to include the context. The query I looked at now doesn't have the context set anymore. Example: ^append$ type:symbol patternType:regexp count:50 case:yes file:\.(go)$ repo:^github.com/sourcegraph/sourcegraph$ index:only I wasn't able to verify all code paths though.
1 parent 9bbdf8c commit 7dbc44a

File tree

7 files changed

+4
-59
lines changed

7 files changed

+4
-59
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ All notable changes to Sourcegraph are documented in this file.
2323
### Changed
2424

2525
- The setting `experimentalFeatures.searchQueryInput` now refers to the new query input as `v2` (not `experimental`). <!-- NOTE: If v2 becomes the default before this release is cut, then update this entry to mention that instead of adding a separate entry. -->
26+
- Search-based code intel doesn't include the currently selected search context anymore. It was possible to get into a situation where search-based code intel wouldn't find any information due to being restricted by the current search context. [#58010](https://github.com/sourcegraph/sourcegraph/pull/58010)
2627

2728
### Fixed
2829

client/shared/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,6 @@ ts_project(
210210
"src/codeintel/legacy-extensions/util/promise.ts",
211211
"src/codeintel/legacy-extensions/util/uri.ts",
212212
"src/codeintel/scip.ts",
213-
"src/codeintel/searchContext.ts",
214213
"src/commands/commands.ts",
215214
"src/components/CodeMirrorEditor.tsx",
216215
"src/components/HighlightedMatches.tsx",

client/shared/src/codeintel/legacy-extensions/util/api.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { once } from 'lodash'
22
import gql from 'tagged-template-noop'
33

4-
import { searchContext } from '../../searchContext'
54
import type * as sourcegraph from '../api'
65
import { cache } from '../util'
76

@@ -68,7 +67,6 @@ export class API {
6867
/**
6968
* Retrieves the name and fork/archive status of a repository. This method
7069
* throws an error if the repository is not known to the Sourcegraph instance.
71-
*
7270
* @param name The repository's name.
7371
*/
7472
public async resolveRepo(name: string): Promise<RepoMeta> {
@@ -298,7 +296,6 @@ export class API {
298296
* Get the content of a file. Throws an error if the repository is not known to
299297
* the Sourcegraph instance. Returns undefined if the input rev or the file is
300298
* not known to the Sourcegraph instance.
301-
*
302299
* @param repo The repository in which the file exists.
303300
* @param revision The revision in which the target version of the file exists.
304301
* @param path The path of the file.
@@ -330,14 +327,10 @@ export class API {
330327

331328
/**
332329
* Perform a search.
333-
*
334330
* @param searchQuery The input to the search command.
335331
* @param fileLocal Set to false to not request this field, which is absent in older versions of Sourcegraph.
336332
*/
337-
public async search(searchQuery: string, fileLocal = true): Promise<SearchResult[]> {
338-
const context = searchContext()
339-
const query = context ? `context:${context} ${searchQuery}` : searchQuery
340-
333+
public async search(query: string, fileLocal = true): Promise<SearchResult[]> {
341334
interface Response {
342335
search: {
343336
results: {

client/shared/src/codeintel/searchContext.ts

Lines changed: 0 additions & 16 deletions
This file was deleted.

client/web/src/LegacySourcegraphWebApp.tsx

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { logger } from '@sourcegraph/common'
1010
import { type GraphQLClient, HTTPStatusError } from '@sourcegraph/http-client'
1111
import { SharedSpanName, TraceSpanProvider } from '@sourcegraph/observability-client'
1212
import { type FetchFileParameters, fetchHighlightedFileLineRanges } from '@sourcegraph/shared/src/backend/file'
13-
import { setCodeIntelSearchContext } from '@sourcegraph/shared/src/codeintel/searchContext'
1413
import type { Controller as ExtensionsController } from '@sourcegraph/shared/src/extensions/controller'
1514
import { createNoopController } from '@sourcegraph/shared/src/extensions/createNoopLoadedController'
1615
import type { PlatformContext } from '@sourcegraph/shared/src/platform/context'
@@ -308,14 +307,6 @@ export class LegacySourcegraphWebApp extends React.Component<StaticAppConfig, Le
308307
}
309308

310309
private async setWorkspaceSearchContext(spec: string | undefined): Promise<void> {
311-
// NOTE(2022-09-08) Inform the inlined code from
312-
// sourcegraph/code-intel-extensions about the change of search context.
313-
// The old extension code previously accessed this information from the
314-
// 'sourcegraph' npm package, and updating the context like this was the
315-
// simplest solution to mirror the old behavior while deprecating
316-
// extensions on a tight deadline. It would be nice to properly pass
317-
// around this via React state in the future.
318-
setCodeIntelSearchContext(spec)
319310
if (this.extensionsController === null) {
320311
return
321312
}

client/web/src/SourcegraphWebApp.tsx

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import { combineLatest, from, Subscription, fromEvent } from 'rxjs'
88

99
import { HTTPStatusError } from '@sourcegraph/http-client'
1010
import { SharedSpanName, TraceSpanProvider } from '@sourcegraph/observability-client'
11-
import { setCodeIntelSearchContext } from '@sourcegraph/shared/src/codeintel/searchContext'
1211
import type { ExtensionsControllerProps } from '@sourcegraph/shared/src/extensions/controller'
1312
import type { PlatformContext } from '@sourcegraph/shared/src/platform/context'
1413
import { ShortcutProvider } from '@sourcegraph/shared/src/react-shortcuts'
@@ -120,22 +119,9 @@ export const SourcegraphWebApp: FC<SourcegraphWebAppProps> = props => {
120119
const [selectedSearchContextSpec, _setSelectedSearchContextSpec] = useState<string | undefined>()
121120

122121
// NOTE(2022-09-08) Inform the inlined code from
123-
// sourcegraph/code-intel-extensions about the change of search context.
124-
// The old extension code previously accessed this information from the
125-
// 'sourcegraph' npm package, and updating the context like this was the
126-
// simplest solution to mirror the old behavior while deprecating
127-
// extensions on a tight deadline. It would be nice to properly pass
128-
// around this via React state in the future.
129-
const setWorkspaceSearchContext = useCallback((spec: string | null): void => {
130-
setCodeIntelSearchContext(spec ?? undefined)
122+
const setSelectedSearchContextSpecWithNoChecks = useCallback((spec: string): void => {
123+
_setSelectedSearchContextSpec(spec)
131124
}, [])
132-
const setSelectedSearchContextSpecWithNoChecks = useCallback(
133-
(spec: string): void => {
134-
_setSelectedSearchContextSpec(spec)
135-
setWorkspaceSearchContext(spec)
136-
},
137-
[setWorkspaceSearchContext]
138-
)
139125
const setSelectedSearchContextSpecToDefault = useCallback((): void => {
140126
if (!props.searchContextsEnabled) {
141127
return
@@ -233,8 +219,6 @@ export const SourcegraphWebApp: FC<SourcegraphWebAppProps> = props => {
233219
setSelectedSearchContextSpecToDefault()
234220
}
235221

236-
setWorkspaceSearchContext(selectedSearchContextSpec ?? null)
237-
238222
return () => subscriptions.unsubscribe()
239223

240224
// We only ever want to run this hook once when the component mounts for

client/web/src/enterprise/codeintel/useSearchBasedCodeIntel.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import type { Range as ExtensionRange, Position as ExtensionPosition } from '@so
99
import { getDocumentNode } from '@sourcegraph/http-client'
1010
import type { LanguageSpec } from '@sourcegraph/shared/src/codeintel/legacy-extensions/language-specs/language-spec'
1111
import { Position as ScipPosition } from '@sourcegraph/shared/src/codeintel/scip'
12-
import { searchContext } from '@sourcegraph/shared/src/codeintel/searchContext'
1312
import { toPrettyBlobURL } from '@sourcegraph/shared/src/util/url'
1413

1514
import { getWebGraphQLClient } from '../../backend/graphql'
@@ -315,7 +314,6 @@ export async function searchBasedDefinitions({
315314
* Perform a search query for definitions. Returns results converted to locations,
316315
* filtered by the language's definition filter, and sorted by proximity to the
317316
* current text document path.
318-
*
319317
* @param api The GraphQL API instance.
320318
* @param args Parameter bag.
321319
*/
@@ -363,11 +361,6 @@ async function searchAndFilterReferences({
363361
}
364362

365363
async function executeSearchQuery(terms: string[]): Promise<SearchResult[]> {
366-
const context = searchContext()
367-
if (context) {
368-
terms.push(`context:${context}`)
369-
}
370-
371364
interface Response {
372365
search: {
373366
results: {

0 commit comments

Comments
 (0)