Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Autocomplete: truncate multiline completions by the next new sibling #1709

Merged
merged 4 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Starting from `0.2.0`, Cody is using `major.EVEN_NUMBER.patch` for release versi
- Command: Fixed an issue that opened a new chat window when running `/doc` and `/edit` commands from the command palette. [pull/1678](https://github.com/sourcegraph/cody/pull/1678)
- Chat: Prevent sidebar from opening when switching editor chat panels. [pull/1691](https://github.com/sourcegraph/cody/pull/1691)
- Chat: Prevent `"command 'cody.chat'panel.new' not found"` error when the new chat panel UI is disabled. [pull/1696](https://github.com/sourcegraph/cody/pull/1696)
- Autocomplete: Improved the multiline completions truncation logic. [pull/1709](https://github.com/sourcegraph/cody/pull/1709)
- Autocomplete: Fix an issue where typing as suggested causes the completion to behave unexpectedly. [pull/1701](https://github.com/sourcegraph/cody/pull/1701)

### Changed
Expand Down
7 changes: 7 additions & 0 deletions vscode/src/completions/get-current-doc-context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ describe('getCurrentDocContext', () => {
nextNonEmptyLine: '',
multilineTrigger: '{',
injectedPrefix: null,
position: { character: 2, line: 1 },
})
})

Expand All @@ -49,6 +50,7 @@ describe('getCurrentDocContext', () => {
nextNonEmptyLine: '}',
multilineTrigger: '{',
injectedPrefix: null,
position: { character: 2, line: 2 },
})
})

Expand All @@ -65,6 +67,7 @@ describe('getCurrentDocContext', () => {
nextNonEmptyLine: '];',
multilineTrigger: '[',
injectedPrefix: null,
position: { character: 13, line: 0 },
})
})

Expand All @@ -81,6 +84,7 @@ describe('getCurrentDocContext', () => {
nextNonEmptyLine: '];',
multilineTrigger: '[',
injectedPrefix: null,
position: { character: 13, line: 1 },
})
})

Expand Down Expand Up @@ -108,6 +112,7 @@ describe('getCurrentDocContext', () => {
nextNonEmptyLine: '',
multilineTrigger: null,
injectedPrefix: 'ssert',
position: { character: 9, line: 0 },
})
})

Expand Down Expand Up @@ -136,6 +141,7 @@ describe('getCurrentDocContext', () => {
nextNonEmptyLine: '',
multilineTrigger: null,
injectedPrefix: 'log',
position: { character: 8, line: 1 },
})
})

Expand Down Expand Up @@ -163,6 +169,7 @@ describe('getCurrentDocContext', () => {
nextNonEmptyLine: '',
multilineTrigger: null,
injectedPrefix: null,
position: { character: 7, line: 0 },
})
})

Expand Down
3 changes: 3 additions & 0 deletions vscode/src/completions/get-current-doc-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ export interface DocumentContext {
multilineTrigger: string | null

completionIntent?: CompletionIntent

position: vscode.Position
}

interface GetCurrentDocContextParams {
Expand Down Expand Up @@ -146,6 +148,7 @@ export function getCurrentDocContext(params: GetCurrentDocContextParams): Docume
nextNonEmptyLine,
injectedPrefix,
completionIntent: completionItent?.name,
position,
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ describe('InlineCompletionItemProvider', () => {
"injectedPrefix": null,
"multilineTrigger": null,
"nextNonEmptyLine": "console.log(1)",
"position": Position {
"character": 12,
"line": 0,
},
"prefix": "const foo = ",
"prevNonEmptyLine": "",
"suffix": "
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { Position, TextDocument } from 'vscode'

import { DocumentContext } from '../get-current-doc-context'
import { getDocumentQuerySDK } from '../tree-sitter/query-sdk'

import { parseCompletion, ParsedCompletion } from './parse-completion'
import { InlineCompletionItemWithAnalytics } from './process-inline-completions'
import { normalizeStartLine, truncateMultilineCompletion } from './truncate-multiline-completion'
import { truncateParsedCompletion } from './truncate-parsed-completion'
import { truncateParsedCompletionByNextSibling } from './truncate-parsed-completion'

export interface ParseAndTruncateParams {
document: TextDocument
Expand All @@ -24,8 +23,7 @@ export function parseAndTruncateCompletion(
document,
multiline,
docContext,
position,
docContext: { prefix, suffix },
docContext: { prefix },
useTreeSitter = true,
} = params

Expand All @@ -34,16 +32,18 @@ export function parseAndTruncateCompletion(
const parsed = parseCompletion({
completion: { insertText: insertTextBeforeTruncation },
document,
position,
docContext,
})

if (parsed.insertText === '') {
return parsed
}

if (multiline) {
const truncationResult = truncateMultilineBlock({
parsed,
document,
suffix,
prefix,
docContext,
useTreeSitter,
})

Expand All @@ -60,9 +60,8 @@ export function parseAndTruncateCompletion(

interface TruncateMultilineBlockParams {
parsed: ParsedCompletion
docContext: DocumentContext
document: TextDocument
prefix: string
suffix: string
useTreeSitter: boolean
}

Expand All @@ -72,16 +71,21 @@ interface TruncateMultilineBlockResult {
}

export function truncateMultilineBlock(params: TruncateMultilineBlockParams): TruncateMultilineBlockResult {
const { parsed, document, prefix, suffix, useTreeSitter } = params
const documentQuerySDK = getDocumentQuerySDK(document.languageId)
const { parsed, docContext, document, useTreeSitter } = params

if (useTreeSitter && parsed.tree && documentQuerySDK) {
if (useTreeSitter && parsed.tree) {
return {
truncatedWith: 'tree-sitter',
insertText: truncateParsedCompletion({ completion: parsed, document, documentQuerySDK }),
insertText: truncateParsedCompletionByNextSibling({
completion: parsed,
docContext,
document,
}),
}
}

const { prefix, suffix } = docContext

return {
truncatedWith: 'indentation',
insertText: truncateMultilineCompletion(parsed.insertText, prefix, suffix, document.languageId),
Expand Down
25 changes: 16 additions & 9 deletions vscode/src/completions/text-processing/parse-completion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import { DocumentContext } from '../get-current-doc-context'
import { asPoint, getCachedParseTreeForDocument } from '../tree-sitter/parse-tree-cache'
import { InlineCompletionItem } from '../types'

import { InlineCompletionItemWithAnalytics } from './process-inline-completions'
import { getMatchingSuffixLength, InlineCompletionItemWithAnalytics } from './process-inline-completions'

export interface CompletionContext {
completion: InlineCompletionItem
document: TextDocument
position: Position
docContext: DocumentContext
}

Expand All @@ -33,7 +32,12 @@ export interface ParsedCompletion extends InlineCompletionItemWithAnalytics {
* would introduce any syntactic errors.
*/
export function parseCompletion(context: CompletionContext): ParsedCompletion {
const { completion, document, position, docContext } = context
const {
completion,
document,
docContext,
docContext: { prefix, position, multilineTrigger },
} = context
const parseTreeCache = getCachedParseTreeForDocument(document)

// Do nothig if the syntactic post-processing is not enabled.
Expand All @@ -45,7 +49,6 @@ export function parseCompletion(context: CompletionContext): ParsedCompletion {
const treeWithCompletion = pasteCompletion({
completion,
document,
position,
docContext,
tree,
parser,
Expand All @@ -64,8 +67,8 @@ export function parseCompletion(context: CompletionContext): ParsedCompletion {
},
}

if (docContext.multilineTrigger) {
const triggerPosition = document.positionAt(docContext.prefix.lastIndexOf(docContext.multilineTrigger))
if (multilineTrigger) {
const triggerPosition = document.positionAt(prefix.lastIndexOf(multilineTrigger))

points.trigger = {
row: triggerPosition.line,
Expand All @@ -89,7 +92,6 @@ export function parseCompletion(context: CompletionContext): ParsedCompletion {
interface PasteCompletionParams {
completion: InlineCompletionItem
document: TextDocument
position: Position
docContext: DocumentContext
tree: Tree
parser: Parser
Expand All @@ -99,19 +101,24 @@ function pasteCompletion(params: PasteCompletionParams): Tree {
const {
completion: { range, insertText },
document,
position,
docContext,
tree,
parser,
docContext: { position, currentLineSuffix },
} = params

const matchingSuffixLength = getMatchingSuffixLength(insertText, currentLineSuffix)

// Adjust suffix and prefix based on completion insert range.
const prefix = range ? document.getText(new Range(new Position(0, 0), range.start as Position)) : docContext.prefix
const suffix = range
? document.getText(new Range(range.end as Position, document.positionAt(document.getText().length)))
: docContext.suffix

const textWithCompletion = prefix + insertText + suffix
// Remove the characters that are being replaced by the completion to avoid having
// them in the parse tree. It breaks the multiline truncation logic which looks for
// the increased number of children in the tree.
const textWithCompletion = prefix + insertText + suffix.slice(matchingSuffixLength)
Comment on lines +118 to +121
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this work in cases where the insertion contains multiple insert blocks and the matchingSuffixLength is not following right after another or am I misunderstanding something? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate a bit more or share an example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of a document state like this:

function bubbleSort(){

and a completion like this:

function bubbleSort(items: any[]): any[] {

But since suffix will be the suffix inside the document, it doesn't matter actually and it should work as expected.


const treeCopy = tree.copy()
const completionEndPosition = position.translate(undefined, insertText.length)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,10 @@ export function getRangeAdjustedForOverlappingCharacters(
item: InlineCompletionItem,
{ position, currentLineSuffix }: AdjustRangeToOverwriteOverlappingCharactersParams
): InlineCompletionItem['range'] {
const matchinSuffixLength = getMatchingSuffixLength(item.insertText, currentLineSuffix)
const matchingSuffixLength = getMatchingSuffixLength(item.insertText, currentLineSuffix)

if (!item.range && currentLineSuffix !== '' && matchinSuffixLength !== 0) {
return new Range(position, position.translate(undefined, matchinSuffixLength))
if (!item.range && currentLineSuffix !== '' && matchingSuffixLength !== 0) {
return new Range(position, position.translate(undefined, matchingSuffixLength))
}

return undefined
Expand Down
Loading