Skip to content

Commit

Permalink
allow contributing actions to location results
Browse files Browse the repository at this point in the history
This lets extensions' location providers (definition/reference/etc. providers) associate context data with each location, and also contribute actions that are shown selectively in the file title for location matches shown in the panel.

Together, these new features allow the basic-code-intel extension to add "Fuzzy" badges to defs/refs in the panel. This fixes #1174.

It would also enable things like:

- Showing a "source" badge on cross-repository reference results explaining how the reference was found (eg by looking up dependents on npm)
- Showing the type of reference (eg denoting some references as "Assignments", some as "Calls", some as "References", etc.)
- Adding an action to "hide" or "always ignore" the reference
- Adding an action to add the match to a saved list
- Adding an action for users to say "This was useful" or "This was not useful" (eg for a code examples extension)

See sourcegraph/code-intel-extensions#10 for an example of how extensions can use this new API.
  • Loading branch information
sqs committed Jan 6, 2019
1 parent c9961a6 commit 3907f04
Show file tree
Hide file tree
Showing 16 changed files with 204 additions and 35 deletions.
1 change: 1 addition & 0 deletions doc/extensions/authoring/contributions.md
Expand Up @@ -23,6 +23,7 @@ A menu is an existing part of the user interface (of Sourcegraph or any other in
* `directory/page`: A section on all pages showing a directory listing. Sometimes known as a "tree page" on code hosts.
* `global/nav`: The global navigation bar, shown at the top of every page.
* `panel/toolbar`: The toolbar on the panel, which is used to show references, definitions, commit history, and other information related to a file or a token/position in a file.
* `location/title`: The title bar of a location result (such as a reference result in the panel shown in response to a "Find references" action).
* `help`: The help menu or page.

The set of available menus is defined in the `menus` property in [`extension.schema.json`](https://sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/shared/src/schema/extension.schema.json).
Expand Down
8 changes: 8 additions & 0 deletions packages/@sourcegraph/extension-api-types/src/location.d.ts
Expand Up @@ -63,4 +63,12 @@ export interface Location {

/** An optional range within the document. */
readonly range?: Range

/**
* Additional data associated with this location. The context data is available to actions
* contributed to the `location` menu. It allows extensions to contribute custom actions to
* locations shown in (e.g.) the references panel, such as an indicator that the location is
* an imprecise match or is on another branch.
*/
readonly context?: ContextValues
}
13 changes: 12 additions & 1 deletion packages/sourcegraph-extension-api/src/sourcegraph.d.ts
Expand Up @@ -311,13 +311,24 @@ declare module 'sourcegraph' {
*/
readonly range?: Range

/**
* Additional data associated with this location. The context data is available to actions
* contributed to the `location` menu. It allows extensions to contribute custom actions to
* locations shown in (e.g.) the references panel, such as an indicator that the location is
* an imprecise match or is on another branch.
*/
readonly context?: Readonly<ContextValues>

/**
* Creates a new location object.
*
* The argument values must not be mutated after the constructor is called.
*
* @param uri The resource identifier.
* @param rangeOrPosition The range or position. Positions will be converted to an empty range.
* @param context Optional context data associated with this location.
*/
constructor(uri: URI, rangeOrPosition?: Range | Position)
constructor(uri: URI, rangeOrPosition?: Range | Position, context?: ContextValues)
}

/**
Expand Down
22 changes: 21 additions & 1 deletion shared/src/actions/ActionItem.tsx
Expand Up @@ -160,6 +160,20 @@ export class ActionItem extends React.PureComponent<Props, State> {
tooltip = this.props.action.description
}

const variantClassName = this.props.variant === 'actionItem' ? 'action-item--variant-action-item' : ''

// Simple display if the action is a noop.
if (!this.props.action.command) {
return (
<span
data-tooltip={tooltip}
className={`action-item ${this.props.className || ''} ${variantClassName}`}
>
{content}
</span>
)
}

const showLoadingSpinner = this.props.showLoadingSpinnerDuringExecution && this.state.actionOrError === LOADING

return (
Expand All @@ -175,7 +189,7 @@ export class ActionItem extends React.PureComponent<Props, State> {
}
className={`action-item ${this.props.className || ''} ${
showLoadingSpinner ? 'action-item--loading' : ''
} ${this.props.variant === 'actionItem' ? 'action-item--variant-action-item' : ''}`}
} ${variantClassName}`}
// If the command is 'open' or 'openXyz' (builtin commands), render it as a link. Otherwise render
// it as a button that executes the command.
to={
Expand All @@ -199,6 +213,12 @@ export class ActionItem extends React.PureComponent<Props, State> {
public runAction = (e: React.MouseEvent<HTMLElement> | React.KeyboardEvent<HTMLElement>) => {
const action = (isAltEvent(e) && this.props.altAction) || this.props.action

if (!action.command) {
// Unexpectedly arrived here; noop actions should not have event handlers that trigger
// this.
return
}

// Record action ID (but not args, which might leak sensitive data).
this.context.log(action.id)

Expand Down
21 changes: 21 additions & 0 deletions shared/src/api/client/context/context.test.ts
Expand Up @@ -218,6 +218,27 @@ describe('getComputedContextProperty', () => {
))
})

describe('location', () => {
test('scoped context shadows outer context', () =>
expect(
getComputedContextProperty(EMPTY_MODEL, EMPTY_SETTINGS_CASCADE, { a: 1 }, 'a', {
type: 'location',
location: { uri: 'x', context: { a: 2 } },
})
).toBe(2))

test('provides location.uri', () =>
expect(
getComputedContextProperty(EMPTY_MODEL, EMPTY_SETTINGS_CASCADE, {}, 'location.uri', {
type: 'location',
location: { uri: 'x' },
})
).toBe('x'))

test('returns null for location.uri when there is no location', () =>
expect(getComputedContextProperty(EMPTY_MODEL, EMPTY_SETTINGS_CASCADE, {}, 'location.uri')).toBe(null))
})

test('falls back to the context entries', () => {
expect(getComputedContextProperty(EMPTY_MODEL, EMPTY_SETTINGS_CASCADE, { x: 1 }, 'x')).toBe(1)
expect(getComputedContextProperty(EMPTY_MODEL, EMPTY_SETTINGS_CASCADE, {}, 'y')).toBe(undefined)
Expand Down
24 changes: 21 additions & 3 deletions shared/src/api/client/context/context.ts
@@ -1,3 +1,4 @@
import { Location } from '@sourcegraph/extension-api-types'
import { basename, dirname, extname } from 'path'
import { isSettingsValid, SettingsCascadeOrError } from '../../../settings/settings'
import { Model, ViewComponentData } from '../model'
Expand Down Expand Up @@ -28,15 +29,16 @@ export function applyContextUpdate(base: Context, update: Context): Context {
*/
export interface Context<T = never>
extends Record<
string,
string | number | boolean | null | Context | T | (string | number | boolean | null | Context | T)[]
> {}
string,
string | number | boolean | null | Context | T | (string | number | boolean | null | Context | T)[]
> {}

export type ContributionScope =
| (Pick<ViewComponentData, 'type' | 'selections'> & {
item: Pick<TextDocumentItem, 'uri' | 'languageId'>
})
| { type: 'panelView'; id: string }
| { type: 'location'; location: Location }

/**
* Looks up a key in the computed context, which consists of computed context properties (with higher precedence)
Expand Down Expand Up @@ -124,6 +126,22 @@ export function getComputedContextProperty(
return component.id
}
}

// Location scope's context shadows outer context.
if (component && component.type === 'location' && component.location.context && key in component.location.context) {
return component.location.context[key]
}
if (key.startsWith('location.')) {
if (!component || component.type !== 'location') {
return null
}
const prop = key.slice('location.'.length)
switch (prop) {
case 'uri':
return component.location.uri
}
}

if (key === 'context') {
return context
}
Expand Down
16 changes: 16 additions & 0 deletions shared/src/api/extension/api/types.test.ts
@@ -0,0 +1,16 @@
import { Location } from '../types/location'
import { Position } from '../types/position'
import { Range } from '../types/range'
import { URI } from '../types/uri'
import { fromLocation } from './types'

describe('fromLocation', () => {
test('converts to location', () =>
expect(
fromLocation(new Location(URI.parse('x'), new Range(new Position(1, 2), new Position(3, 4)), { a: 1 }))
).toEqual({
uri: 'x',
range: { start: { line: 1, character: 2 }, end: { line: 3, character: 4 } },
context: { a: 1 },
}))
})
1 change: 1 addition & 0 deletions shared/src/api/extension/api/types.ts
Expand Up @@ -21,6 +21,7 @@ export function fromLocation(location: sourcegraph.Location): clientType.Locatio
return {
uri: location.uri.toString(),
range: fromRange(location.range),
context: location.context,
}
}

Expand Down
7 changes: 7 additions & 0 deletions shared/src/api/extension/types/location.test.ts
Expand Up @@ -9,10 +9,17 @@ describe('Location', () => {
assertToJSON(new Location(URI.file('u.ts'), new Position(3, 4)), {
uri: URI.parse('file://u.ts').toJSON(),
range: { start: { line: 3, character: 4 }, end: { line: 3, character: 4 } },
context: undefined,
})
assertToJSON(new Location(URI.file('u.ts'), new Range(1, 2, 3, 4)), {
uri: URI.parse('file://u.ts').toJSON(),
range: { start: { line: 1, character: 2 }, end: { line: 3, character: 4 } },
context: undefined,
})
assertToJSON(new Location(URI.file('u.ts'), new Range(1, 2, 3, 4), { a: 1 }), {
uri: URI.parse('file://u.ts').toJSON(),
range: { start: { line: 1, character: 2 }, end: { line: 3, character: 4 } },
context: { a: 1 },
})
})
})
7 changes: 6 additions & 1 deletion shared/src/api/extension/types/location.ts
Expand Up @@ -16,7 +16,11 @@ export class Location implements sourcegraph.Location {

public readonly range?: sourcegraph.Range

constructor(public readonly uri: sourcegraph.URI, rangeOrPosition?: sourcegraph.Range | sourcegraph.Position) {
constructor(
public readonly uri: sourcegraph.URI,
rangeOrPosition?: sourcegraph.Range | sourcegraph.Position,
public readonly context?: sourcegraph.ContextValues
) {
if (!rangeOrPosition) {
// that's OK
} else if (rangeOrPosition instanceof Range) {
Expand All @@ -32,6 +36,7 @@ export class Location implements sourcegraph.Location {
return {
uri: this.uri,
range: this.range,
context: this.context,
}
}
}
25 changes: 17 additions & 8 deletions shared/src/api/protocol/contribution.ts
Expand Up @@ -34,22 +34,26 @@ export interface ActionContribution {
id: string

/**
* The command that this action invokes. It can refer to a command registered by the same extension or any
* other extension, or to a builtin command.
* The command that this action invokes. It can refer to a command registered by the same
* extension or any other extension, or to a builtin command. If it is undefined, the action is
* a noop.
*
* See "[Builtin commands](../../../../doc/extensions/authoring/builtin_commands.md)" (online at
* https://docs.sourcegraph.com/extensions/authoring/builtin_commands) for documentation on builtin client
* commands.
* https://docs.sourcegraph.com/extensions/authoring/builtin_commands) for documentation on
* builtin client commands.
*
* Extensions: The command must be registered (unless it is a builtin command). Extensions can register
* commands using {@link sourcegraph.commands.registerCommand}.
* Extensions: The command must be registered (unless it is a builtin command). Extensions can
* register commands using {@link sourcegraph.commands.registerCommand}.
*
* Clients: All clients must implement the builtin commands as specified in the documentation above.
* Clients: All clients must implement the builtin commands as specified in the documentation
* above. If the command is undefined (which means the action is a noop), the action should be
* displayed in such a way that avoids suggesting it can be clicked/activated (e.g., there
* should not be any hover or active state on the button).
*
* @see ActionContributionClientCommandOpen
* @see ActionContributionClientCommandUpdateConfiguration
*/
command: string
command?: string

/**
* Optional arguments to pass to the extension when the action is invoked.
Expand Down Expand Up @@ -203,6 +207,11 @@ export enum ContributableMenu {
/** The panel toolbar. */
PanelToolbar = 'panel/toolbar',

/**
* The title bar of a location result, such as a reference or definition location in the panel.
*/
LocationTitle = 'location/title',

/** The help menu in the application. */
Help = 'help',
}
Expand Down
22 changes: 16 additions & 6 deletions shared/src/components/FileMatch.tsx
Expand Up @@ -57,6 +57,11 @@ interface Props {
*/
showAllMatches: boolean

/**
* An extra React fragment to render in the header.
*/
extraHeader?: React.ReactFragment

isLightTheme: boolean

allExpanded?: boolean
Expand Down Expand Up @@ -91,12 +96,17 @@ export class FileMatch extends React.PureComponent<Props> {
}))

const title = (
<RepoFileLink
repoName={result.repository.name}
repoURL={result.repository.url}
filePath={result.file.path}
fileURL={result.file.url}
/>
<div className="d-flex align-items-center justify-content-between">
<div>
<RepoFileLink
repoName={result.repository.name}
repoURL={result.repository.url}
filePath={result.file.path}
fileURL={result.file.url}
/>
</div>
{this.props.extraHeader ? <div>{this.props.extraHeader}</div> : null}
</div>
)

let containerProps: ResultContainerProps
Expand Down
1 change: 1 addition & 0 deletions shared/src/panel/Panel.tsx
Expand Up @@ -83,6 +83,7 @@ export class Panel extends React.PureComponent<Props, State> {
location={this.props.location}
isLightTheme={this.props.isLightTheme}
extensionsController={this.props.extensionsController}
platformContext={this.props.platformContext}
settingsCascade={this.props.settingsCascade}
fetchHighlightedFileLines={this.props.fetchHighlightedFileLines}
/>
Expand Down
36 changes: 34 additions & 2 deletions shared/src/panel/views/FileLocations.tsx
@@ -1,16 +1,21 @@
import { Location } from '@sourcegraph/extension-api-types'
import { LoadingSpinner } from '@sourcegraph/react-loading-spinner'
import * as H from 'history'
import { upperFirst } from 'lodash'
import AlertCircleIcon from 'mdi-react/AlertCircleIcon'
import MapSearchIcon from 'mdi-react/MapSearchIcon'
import * as React from 'react'
import { Observable, Subject, Subscription } from 'rxjs'
import { catchError, distinctUntilChanged, map, startWith, switchMap } from 'rxjs/operators'
import { ActionsNavItems } from '../../actions/ActionsNavItems'
import { ContributableMenu } from '../../api/protocol'
import { FetchFileCtx } from '../../components/CodeExcerpt'
import { FileMatch, IFileMatch, ILineMatch } from '../../components/FileMatch'
import { VirtualList } from '../../components/VirtualList'
import { asError } from '../../util/errors'
import { ExtensionsControllerProps } from '../../extensions/controller'
import { PlatformContextProps } from '../../platform/context'
import { ErrorLike, isErrorLike } from '../../util/errors'
import { asError } from '../../util/errors'
import { propertyIsDefined } from '../../util/types'
import { parseRepoURI, toPrettyBlobURL } from '../../util/url'

Expand All @@ -26,7 +31,9 @@ export const FileLocationsNotFound: React.FunctionComponent = () => (
</div>
)

interface Props {
interface Props extends ExtensionsControllerProps, PlatformContextProps {
location: H.Location

/**
* The observable that emits the locations.
*/
Expand Down Expand Up @@ -135,6 +142,31 @@ export class FileLocations extends React.PureComponent<Props, State> {
key={i}
expanded={true}
result={refsToFileMatch(uri, locationsByURI.get(uri)!)}
extraHeader={
<ActionsNavItems
listClass="align-items-center justify-content-end"
actionItemClass="badge badge-secondary"
menu={ContributableMenu.LocationTitle}
extensionsController={this.props.extensionsController}
platformContext={this.props.platformContext}
location={this.props.location}
scope={{
type: 'location',
// KNOWN ISSUE: If multiple matches are found in a file, it
// only shows the contributed actions for the first
// location. (E.g., if the first match in a file is fuzzy
// and all the rest aren't, and you're using the
// basic-code-intel extension, then the "Fuzzy" badge
// appears in the file header and appears to apply to all
// matches in the file, not just the first.) To fix this, we
// would need to rethink how matches are displayed (VS
// Code's way of displaying matches would make it possible
// for us to avoid this problem).
location: locationsByURI.get(uri)![0],
}}
wrapInList={true}
/>
}
icon={this.props.icon}
onSelect={this.onSelect}
showAllMatches={true}
Expand Down

0 comments on commit 3907f04

Please sign in to comment.