Skip to content

Commit

Permalink
Chat: improve @mentions on input (#3606)
Browse files Browse the repository at this point in the history
  • Loading branch information
abeatrix authored Apr 1, 2024
1 parent 4fc1e44 commit a2133af
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 31 deletions.
2 changes: 1 addition & 1 deletion lib/shared/src/mentions/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const VALID_CHARS = '[^' + TRIGGERS + PUNCTUATION + '\\s]'

const MAX_LENGTH = 250

const RANGE_REGEXP = '(?::\\d+-\\d+)?'
const RANGE_REGEXP = '(?::\\d+(?:-\\d*)?)?'

const AT_MENTIONS_REGEXP = new RegExp(
'(?<maybeLeadingWhitespace>^|\\s|\\()(?<replaceableString>' +
Expand Down
14 changes: 14 additions & 0 deletions vscode/test/e2e/chat-atFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ test.extend<ExpectedEvents>({
await chatInput.fill('Explain @mj')
await chatPanelFrame.getByRole('option', { name: 'Main.java' }).click()
await expect(chatInput).toHaveText('Explain @Main.java ')
await expect(chatInput.getByText('@Main.java')).not.toHaveClass(/context-item-mention-node/)
await chatInput.press('Enter')
await expect(chatInput.getByText('@Main.java')).toHaveClass(/context-item-mention-node/)
await chatInput.press('Enter')
await expect(chatInput).toBeEmpty()
Expand All @@ -102,6 +104,7 @@ test.extend<ExpectedEvents>({
chatPanelFrame.getByRole('option', { name: withPlatformSlashes('var.go lib/batches/env') })
).toBeVisible()
await chatInput.press('Tab')
await chatInput.press('Tab')
await expect(chatInput).toHaveText(withPlatformSlashes('Explain @lib/batches/env/var.go '))
await chatInput.focus()
await chatInput.pressSequentially('and ')
Expand All @@ -115,6 +118,7 @@ test.extend<ExpectedEvents>({
await chatInput.press('ArrowDown') // second item again
await expect(chatPanelFrame.getByRole('option', { selected: true })).toHaveText(/visualize\.go/)
await chatInput.press('Tab')
await chatInput.press('Tab')
await expect(chatInput).toHaveText(
withPlatformSlashes(
'Explain @lib/batches/env/var.go and @lib/codeintel/tools/lsif-visualize/visualize.go '
Expand Down Expand Up @@ -142,13 +146,15 @@ test.extend<ExpectedEvents>({
await chatInput.pressSequentially('@Main.java', { delay: 10 })
await expect(chatPanelFrame.getByRole('option', { name: 'Main.java' })).toBeVisible()
await chatInput.press('Tab')
await chatInput.press('Tab')
await expect(chatInput).toHaveText('@Main.java ')

// Check pressing tab after typing a partial filename but where that complete
// filename already exists earlier in the input.
// https://github.com/sourcegraph/cody/issues/2243
await chatInput.pressSequentially('and @Main.ja', { delay: 10 })
await chatInput.press('Tab')
await chatInput.press('Tab')
await expect(chatInput).toHaveText('@Main.java and @Main.java ')

// Support @-file in mid-sentence
Expand All @@ -164,6 +170,7 @@ test.extend<ExpectedEvents>({
await chatInput.pressSequentially('@Main', { delay: 10 })
await expect(chatPanelFrame.getByRole('option', { name: 'Main.java' })).toBeVisible()
await chatInput.press('Tab')
await chatInput.press('Tab')
await expect(chatInput).toHaveText('Explain the @Main.java file')
// Confirm the cursor is at the end of the newly added file name with space
await page.keyboard.press('!')
Expand Down Expand Up @@ -208,6 +215,7 @@ test('editing a chat message with @-mention', async ({ page, sidebar }) => {
// Send a message with an @-mention.
await chatInput.fill('Explain @mj')
await chatPanelFrame.getByRole('option', { name: 'Main.java' }).click()
await chatInput.press('Enter')
await expect(chatInput).toHaveText('Explain @Main.java ')
await expect(chatInput.getByText('@Main.java')).toHaveClass(/context-item-mention-node/)
await chatInput.press('Enter')
Expand All @@ -227,6 +235,7 @@ test('editing a chat message with @-mention', async ({ page, sidebar }) => {
await expect(chatInput).toHaveText('Explain @Main.java ')
await chatInput.pressSequentially('and @index.ht')
await chatPanelFrame.getByRole('option', { name: 'index.html' }).click()
await chatPanelFrame.getByRole('option', { name: 'index.html' }).click()
await expect(chatInput).toHaveText('Explain @Main.java and @index.html')
await expect(chatInput.getByText('@index.html')).toHaveClass(/context-item-mention-node/)
await chatInput.press('Enter')
Expand All @@ -245,6 +254,7 @@ test('pressing Enter with @-mention menu open selects item, does not submit mess
await chatInput.fill('Explain @index.htm')
await expect(chatPanelFrame.getByRole('option', { name: 'index.html' })).toBeVisible()
await chatInput.press('Enter')
await chatInput.press('Enter')
await expect(chatInput).toHaveText('Explain @index.html')
await expect(chatInput.getByText('@index.html')).toHaveClass(/context-item-mention-node/)
})
Expand All @@ -259,6 +269,7 @@ test('@-mention links in transcript message', async ({ page, sidebar }) => {
await chatInput.fill('Hello @buzz.ts')
await chatPanelFrame.getByRole('option', { name: 'buzz.ts' }).click()
await chatInput.press('Enter')
await chatInput.press('Enter')

// In the transcript, the @-mention is linked, and clicking the link opens the file.
const transcriptMessage = chatPanelFrame.getByText('Hello @buzz.ts')
Expand All @@ -281,6 +292,8 @@ test('@-mention file range', async ({ page, sidebar }) => {
await expect(chatPanelFrame.getByRole('option', { name: 'buzz.ts Lines 2-4' })).toBeVisible()
await chatPanelFrame.getByRole('option', { name: 'buzz.ts Lines 2-4' }).click()
await expect(chatInput).toHaveText('@buzz.ts:2-4 ')
// Enter again to turn it into a token
await chatInput.press('Enter')

// Submit the message
await chatInput.press('Enter')
Expand All @@ -298,6 +311,7 @@ test('@-mention file range', async ({ page, sidebar }) => {
await expect(previewTab).toBeVisible()
})

// NOTE: @symbols does not require double tabbing to select an option.
test.extend<ExpectedEvents>({
expectedEvents: ['CodyVSCodeExtension:at-mention:symbol:executed'],
})('@-mention symbol in chat', async ({ page, sidebar }) => {
Expand Down
2 changes: 2 additions & 0 deletions vscode/test/e2e/chat-edits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ test.extend<ExpectedEvents>({
await expect(chatInput).not.toHaveText('Four')
await expect(chatFrame.getByRole('option', { name: 'Main.java' })).toBeVisible()
await chatInput.press('Tab')
await chatInput.press('Tab')
await expect(chatInput).toHaveText('Explain @Main.java ')

// Enter should submit the message and exit editing mode
Expand All @@ -121,6 +122,7 @@ test.extend<ExpectedEvents>({
await expect(chatInput).toHaveText('Explain @Main.java ')
await chatInput.type('and @vgo', { delay: 50 })
await chatInput.press('Tab')
await chatInput.press('Tab')
await expect(chatInput).toHaveText(
withPlatformSlashes('Explain @Main.java and @lib/batches/env/var.go ')
)
Expand Down
11 changes: 10 additions & 1 deletion vscode/webviews/promptEditor/nodes/ContextItemMentionNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,13 @@ export function contextItemMentionNodeDisplayText(contextItem: SerializedContext
// range needs to go to the start (0th character) of line 5. Also, `RangeData` is 0-indexed but
// display ranges are 1-indexed.
const rangeText = contextItem.range ? `:${displayLineRange(contextItem.range)}` : ''
// Large file cannot be added to the context file list.
if (contextItem.type === 'file' && !contextItem.isTooLarge) {
return `@${displayPath(URI.parse(contextItem.uri))}${rangeText}`
}
// Large file cannot be added to the context file list unless it has a range.
if (contextItem.type === 'file' && contextItem.isTooLarge && rangeText) {
return `@${displayPath(URI.parse(contextItem.uri))}${rangeText}`
}
if (contextItem.type === 'symbol') {
return `@${displayPath(URI.parse(contextItem.uri))}${rangeText}#${contextItem.symbolName}`
}
Expand All @@ -206,3 +209,9 @@ export function isSerializedContextItemMentionNode(
): node is SerializedContextItemMentionNode {
return Boolean(node && node.type === ContextItemMentionNode.getType())
}

export function $createContextItemTextNode(contextItem: ContextItem | SerializedContextItem): TextNode {
const atNode = new ContextItemMentionNode(contextItem)
const textNode = new TextNode(atNode.__text)
return $applyNodeReplacement(textNode)
}
46 changes: 24 additions & 22 deletions vscode/webviews/promptEditor/plugins/atMentions/OptionsList.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { MenuRenderFn } from '@lexical/react/LexicalTypeaheadMenuPlugin'
import {
type RangeData,
displayLineRange,
displayPath,
displayPathBasename,
Expand All @@ -18,7 +19,7 @@ import {
SYMBOL_HELP_LABEL,
} from '../../../../src/chat/context/constants'
import styles from './OptionsList.module.css'
import type { MentionTypeaheadOption } from './atMentions'
import { type MentionTypeaheadOption, RANGE_MATCHES_REGEXP } from './atMentions'

export const OptionsList: FunctionComponent<
{ query: string; options: MentionTypeaheadOption[] } & Pick<
Expand All @@ -31,26 +32,9 @@ export const OptionsList: FunctionComponent<
useEffect(() => {
// Scroll to top when options change because the prior `selectedIndex` is invalidated.
ref?.current?.scrollTo(0, 0)
const validIndex = options.findIndex(o => o?.item?.type === 'file' && !o?.item?.isTooLarge)
setHighlightedIndex(validIndex < 0 ? 0 : validIndex)
setHighlightedIndex(0)
}, [options])

// biome-ignore lint/correctness/useExhaustiveDependencies: Intent is to run whenever `selectedIndex` changes.
useEffect(() => {
if (selectedIndex === null) {
return
}
// If the selectedIndex isTooLarge, set it to the next valid option.
const current = options[selectedIndex]
const currentOptionIsInvalid = current?.item?.type === 'file' && current?.item?.isTooLarge
if (currentOptionIsInvalid) {
const validIndex = options.findIndex(
(o, i) => i > selectedIndex && o?.item?.type === 'file' && !o?.item?.isTooLarge
)
setHighlightedIndex(validIndex)
}
}, [selectedIndex])

const mentionQuery = parseMentionQuery(query)

return (
Expand All @@ -74,6 +58,7 @@ export const OptionsList: FunctionComponent<
<ul ref={ref} className={styles.list}>
{options.map((option, i) => (
<Item
query={query}
isSelected={selectedIndex === i}
onClick={() => {
setHighlightedIndex(i)
Expand All @@ -94,17 +79,18 @@ export const OptionsList: FunctionComponent<
}

const Item: FunctionComponent<{
query: string
isSelected: boolean
onClick: () => void
onMouseEnter: () => void
option: MentionTypeaheadOption
className?: string
}> = ({ isSelected, onClick, onMouseEnter, option, className }) => {
}> = ({ query, isSelected, onClick, onMouseEnter, option, className }) => {
const item = option.item
const icon =
item.type === 'file' ? null : item.kind === 'class' ? 'symbol-structure' : 'symbol-method'
const title = item.title ?? (item.type === 'file' ? displayPathBasename(item.uri) : item.symbolName)
const range = item.range ? displayLineRange(item.range) : ''
const range = getLineRangeInMention(query, item.range)
const dirname = displayPathDirname(item.uri)
const description =
item.type === 'file'
Expand All @@ -120,7 +106,7 @@ const Item: FunctionComponent<{
className={classNames(
className,
styles.optionItem,
isSelected && !warning && styles.selected,
isSelected && styles.selected,
warning && styles.disabled
)}
ref={option.setRefElement}
Expand Down Expand Up @@ -148,3 +134,19 @@ const Item: FunctionComponent<{
</li>
)
}

/**
* Gets the display line range from the query string.
*/
function getLineRangeInMention(query: string, range?: RangeData): string {
// Parses out the start and end line numbers from the query if it contains a line range match.
const queryRange = query.match(RANGE_MATCHES_REGEXP)
if (query && queryRange?.[1]) {
const [_, start, end] = queryRange
const startLine = parseInt(start)
const endLine = end ? parseInt(end) : Number.POSITIVE_INFINITY
return `${startLine}-${endLine !== Number.POSITIVE_INFINITY ? endLine : '#'}`
}
// Passed in range string if no line number match.
return range ? displayLineRange(range) : ''
}
29 changes: 23 additions & 6 deletions vscode/webviews/promptEditor/plugins/atMentions/atMentions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@ import {
scanForMentionTriggerInUserTextInput,
} from '@sourcegraph/cody-shared'
import classNames from 'classnames'
import { $createContextItemMentionNode } from '../../nodes/ContextItemMentionNode'
import {
$createContextItemMentionNode,
$createContextItemTextNode,
} from '../../nodes/ContextItemMentionNode'
import { OptionsList } from './OptionsList'
import { useChatContextItems } from './chatContextClient'

export const RANGE_MATCHES_REGEXP = /:(\d+)?-?(\d+)?$/
export const LINE_RANGE_REGEXP = /:(\d+)-(\d+)$/

/**
* Parses the line range (if any) at the end of a string like `foo.txt:1-2`. Because this means "all
* of lines 1 and 2", the returned range actually goes to the start of line 3 to ensure all of line
Expand All @@ -31,7 +37,7 @@ export function parseLineRangeInMention(text: string): {
textWithoutRange: string
range?: RangeData
} {
const match = text.match(/:(\d+)-(\d+)$/)
const match = text.match(LINE_RANGE_REGEXP)
if (match === null) {
return { textWithoutRange: text }
}
Expand Down Expand Up @@ -101,15 +107,26 @@ export default function MentionsPlugin(): JSX.Element | null {
closeMenu: () => void
) => {
editor.update(() => {
const mentionNode = $createContextItemMentionNode(selectedOption.item)
if (nodeToReplace) {
nodeToReplace.replace(mentionNode)
const currentInputText = nodeToReplace?.__text
if (!currentInputText) {
return
}
// On first selection, add the selected option as text.
// This allows users to autocomplete the file path, and provide them with
// the options to make additional changes, e.g. add range, before inserting the mention.
const textNode = $createContextItemTextNode(selectedOption.item)
if (!currentInputText.endsWith(textNode.__text) && !currentInputText.startsWith('@#')) {
nodeToReplace.replace(textNode)
textNode.select()
closeMenu()
return
}

const mentionNode = $createContextItemMentionNode(selectedOption.item)
nodeToReplace?.replace(mentionNode)
const spaceAfter = $createTextNode(' ')
mentionNode.insertAfter(spaceAfter)
spaceAfter.select()

closeMenu()
})
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ContextItem } from '@sourcegraph/cody-shared'
import { type FunctionComponent, createContext, useContext, useEffect, useState } from 'react'
import { getVSCodeAPI } from '../../../utils/VSCodeApi'
import { parseLineRangeInMention } from './atMentions'
import { LINE_RANGE_REGEXP, RANGE_MATCHES_REGEXP, parseLineRangeInMention } from './atMentions'

export interface ChatContextClient {
getChatContextItems(query: string): Promise<ContextItem[]>
Expand Down Expand Up @@ -66,6 +66,13 @@ export function useChatContextItems(query: string | null): ContextItem[] | undef
return
}

// If user is typing a line range, no need to fetch new chat context items.
if (results?.length && RANGE_MATCHES_REGEXP.test(query)) {
if (!LINE_RANGE_REGEXP.test(query)) {
return
}
}

// Track if the query changed since this request was sent (which would make our results
// no longer valid).
let invalidated = false
Expand Down

0 comments on commit a2133af

Please sign in to comment.