Fix range when copying empty selection (#182227)

This fixes the range extensions get when copying an empty selection. As part of this, I've also:

- Added tests for this change
- Made the paste parts of the api optional. This is useful when a test provider only wants to add data on copy
This commit is contained in:
Matt Bierner
2023-05-11 17:31:11 -07:00
committed by GitHub
parent 068cbf3133
commit 9b6b547d2d
9 changed files with 241 additions and 30 deletions

View File

@@ -10,6 +10,7 @@
"customEditorMove", "customEditorMove",
"diffCommand", "diffCommand",
"documentFiltersExclusive", "documentFiltersExclusive",
"documentPaste",
"editorInsets", "editorInsets",
"extensionRuntime", "extensionRuntime",
"extensionsAny", "extensionsAny",

View File

@@ -0,0 +1,190 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import * as assert from 'assert';
import * as vscode from 'vscode';
import { assertNoRpc, createRandomFile, usingDisposables } from '../utils';
const textPlain = 'text/plain';
(vscode.env.uiKind === vscode.UIKind.Web ? suite.skip : suite)('vscode API - Copy Paste', () => {
teardown(assertNoRpc);
test('Copy should be able to overwrite text/plain', usingDisposables(async (disposables) => {
const file = await createRandomFile('$abcde@');
const doc = await vscode.workspace.openTextDocument(file);
const editor = await vscode.window.showTextDocument(doc);
editor.selections = [new vscode.Selection(0, 1, 0, 6)];
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
const existing = dataTransfer.get(textPlain);
if (existing) {
const str = await existing.asString();
const reversed = reverseString(str);
dataTransfer.set(textPlain, new vscode.DataTransferItem(reversed));
}
}
}, { copyMimeTypes: [textPlain] }));
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
const newDocContent = getNextDocumentText(disposables, doc);
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
assert.strictEqual(await newDocContent, '$edcba@');
}));
test('Copy with empty selection should copy entire line', usingDisposables(async (disposables) => {
const file = await createRandomFile('abc\ndef');
const doc = await vscode.workspace.openTextDocument(file);
await vscode.window.showTextDocument(doc);
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
const existing = dataTransfer.get(textPlain);
if (existing) {
const str = await existing.asString();
// text/plain includes the trailing new line in this case
// On windows, this will always be `\r\n` even if the document uses `\n`
const eol = str.match(/\r?\n$/);
const reversed = reverseString(str.slice(0, -eol![0].length));
dataTransfer.set(textPlain, new vscode.DataTransferItem(reversed + '\n'));
}
}
}, { copyMimeTypes: [textPlain] }));
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
const newDocContent = getNextDocumentText(disposables, doc);
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
assert.strictEqual(await newDocContent, `cba\nabc\ndef`);
}));
test('Copy with multiple selections should get all selections', usingDisposables(async (disposables) => {
const file = await createRandomFile('111\n222\n333');
const doc = await vscode.workspace.openTextDocument(file);
const editor = await vscode.window.showTextDocument(doc);
editor.selections = [
new vscode.Selection(0, 0, 0, 3),
new vscode.Selection(2, 0, 2, 3),
];
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
async prepareDocumentPaste(document: vscode.TextDocument, ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
const existing = dataTransfer.get(textPlain);
if (existing) {
const selections = ranges.map(range => document.getText(range));
dataTransfer.set(textPlain, new vscode.DataTransferItem(`(${ranges.length})${selections.join(' ')}`));
}
}
}, { copyMimeTypes: [textPlain] }));
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
editor.selections = [new vscode.Selection(0, 0, 0, 0)];
const newDocContent = getNextDocumentText(disposables, doc);
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
assert.strictEqual(await newDocContent, `(2)111 333111\n222\n333`);
}));
test('Earlier invoked copy providers should win when writing values', usingDisposables(async (disposables) => {
const file = await createRandomFile('abc\ndef');
const doc = await vscode.workspace.openTextDocument(file);
const editor = await vscode.window.showTextDocument(doc);
editor.selections = [new vscode.Selection(0, 0, 0, 3)];
const callOrder: string[] = [];
const a_id = 'a';
const b_id = 'b';
let providerAResolve: () => void;
const providerAFinished = new Promise<void>(resolve => providerAResolve = resolve);
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
callOrder.push(a_id);
dataTransfer.set(textPlain, new vscode.DataTransferItem('a'));
providerAResolve();
}
}, { copyMimeTypes: [textPlain] }));
// Later registered providers will be called first
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
callOrder.push(b_id);
// Wait for the first provider to finish even though we were called first.
// This tests that resulting order does not depend on the order the providers
// return in.
await providerAFinished;
dataTransfer.set(textPlain, new vscode.DataTransferItem('b'));
}
}, { copyMimeTypes: [textPlain] }));
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
const newDocContent = getNextDocumentText(disposables, doc);
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
assert.strictEqual(await newDocContent, 'b\ndef');
// Confirm provider call order is what we expected
assert.deepStrictEqual(callOrder, [b_id, a_id]);
}));
test('Copy providers should not be able to effect the data transfer of another', usingDisposables(async (disposables) => {
const file = await createRandomFile('abc\ndef');
const doc = await vscode.workspace.openTextDocument(file);
const editor = await vscode.window.showTextDocument(doc);
editor.selections = [new vscode.Selection(0, 0, 0, 3)];
let providerAResolve: () => void;
const providerAFinished = new Promise<void>(resolve => providerAResolve = resolve);
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
dataTransfer.set(textPlain, new vscode.DataTransferItem('xyz'));
providerAResolve();
}
}, { copyMimeTypes: [textPlain] }));
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
// Wait for the first provider to finish
await providerAFinished;
// We we access the data transfer here, we should not see changes made by the first provider
const entry = dataTransfer.get(textPlain);
const str = await entry!.asString();
dataTransfer.set(textPlain, new vscode.DataTransferItem(reverseString(str)));
}
}, { copyMimeTypes: [textPlain] }));
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
const newDocContent = getNextDocumentText(disposables, doc);
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
assert.strictEqual(await newDocContent, 'cba\ndef');
}));
});
function reverseString(str: string) {
return str.split("").reverse().join("");
}
function getNextDocumentText(disposables: vscode.Disposable[], doc: vscode.TextDocument): Promise<string> {
return new Promise<string>(resolve => {
disposables.push(vscode.workspace.onDidChangeTextDocument(e => {
if (e.document === doc) {
resolve(doc.getText());
}
}));
});
}

View File

@@ -62,6 +62,17 @@ export function disposeAll(disposables: vscode.Disposable[]) {
vscode.Disposable.from(...disposables).dispose(); vscode.Disposable.from(...disposables).dispose();
} }
export function usingDisposables<R>(fn: (this: Mocha.Context, store: vscode.Disposable[]) => Promise<R>) {
return async function (this: Mocha.Context): Promise<R> {
const disposables: vscode.Disposable[] = [];
try {
return await fn.call(this, disposables);
} finally {
disposeAll(disposables);
}
};
}
export function delay(ms: number) { export function delay(ms: number) {
return new Promise(resolve => setTimeout(resolve, ms)); return new Promise(resolve => setTimeout(resolve, ms));
} }

View File

@@ -799,11 +799,11 @@ export interface DocumentPasteEditProvider {
readonly id?: string; readonly id?: string;
readonly copyMimeTypes?: readonly string[]; readonly copyMimeTypes?: readonly string[];
readonly pasteMimeTypes: readonly string[]; readonly pasteMimeTypes?: readonly string[];
prepareDocumentPaste?(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<undefined | IReadonlyVSDataTransfer>; prepareDocumentPaste?(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<undefined | IReadonlyVSDataTransfer>;
provideDocumentPasteEdits(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>; provideDocumentPasteEdits?(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
} }
/** /**

View File

@@ -133,7 +133,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
return; return;
} }
ranges = ranges.map(range => new Range(range.startLineNumber, 0, range.startLineNumber, model.getLineLength(range.startLineNumber))); ranges = [new Range(ranges[0].startLineNumber, 1, ranges[0].startLineNumber, 1 + model.getLineLength(ranges[0].startLineNumber))];
} }
const toCopy = this._editor._getViewModel()?.getPlainTextToCopy(selections, enableEmptySelectionClipboard, platform.isWindows); const toCopy = this._editor._getViewModel()?.getPlainTextToCopy(selections, enableEmptySelectionClipboard, platform.isWindows);
@@ -219,7 +219,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
const allProviders = this._languageFeaturesService.documentPasteEditProvider const allProviders = this._languageFeaturesService.documentPasteEditProvider
.ordered(model) .ordered(model)
.filter(provider => provider.pasteMimeTypes.some(type => matchesMimeType(type, allPotentialMimeTypes))); .filter(provider => provider.pasteMimeTypes?.some(type => matchesMimeType(type, allPotentialMimeTypes)));
if (!allProviders.length) { if (!allProviders.length) {
return; return;
} }
@@ -253,7 +253,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
} }
// Filter out any providers the don't match the full data transfer we will send them. // Filter out any providers the don't match the full data transfer we will send them.
const supportedProviders = allProviders.filter(provider => isSupportedProvider(provider, dataTransfer)); const supportedProviders = allProviders.filter(provider => isSupportedPasteProvider(provider, dataTransfer));
if (!supportedProviders.length if (!supportedProviders.length
|| (supportedProviders.length === 1 && supportedProviders[0].id === 'text') // Only our default text provider is active || (supportedProviders.length === 1 && supportedProviders[0].id === 'text') // Only our default text provider is active
) { ) {
@@ -300,7 +300,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
} }
// Filter out any providers the don't match the full data transfer we will send them. // Filter out any providers the don't match the full data transfer we will send them.
const supportedProviders = allProviders.filter(provider => isSupportedProvider(provider, dataTransfer)); const supportedProviders = allProviders.filter(provider => isSupportedPasteProvider(provider, dataTransfer));
const providerEdits = await this.getPasteEdits(supportedProviders, dataTransfer, model, selections, tokenSource.token); const providerEdits = await this.getPasteEdits(supportedProviders, dataTransfer, model, selections, tokenSource.token);
if (tokenSource.token.isCancellationRequested) { if (tokenSource.token.isCancellationRequested) {
@@ -392,7 +392,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
private async getPasteEdits(providers: readonly DocumentPasteEditProvider[], dataTransfer: VSDataTransfer, model: ITextModel, selections: readonly Selection[], token: CancellationToken): Promise<DocumentPasteEdit[]> { private async getPasteEdits(providers: readonly DocumentPasteEditProvider[], dataTransfer: VSDataTransfer, model: ITextModel, selections: readonly Selection[], token: CancellationToken): Promise<DocumentPasteEdit[]> {
const result = await raceCancellation( const result = await raceCancellation(
Promise.all( Promise.all(
providers.map(provider => provider.provideDocumentPasteEdits(model, selections, dataTransfer, token)) providers.map(provider => provider.provideDocumentPasteEdits?.(model, selections, dataTransfer, token))
).then(coalesce), ).then(coalesce),
token); token);
result?.sort((a, b) => b.priority - a.priority); result?.sort((a, b) => b.priority - a.priority);
@@ -420,6 +420,6 @@ export class CopyPasteController extends Disposable implements IEditorContributi
} }
} }
function isSupportedProvider(provider: DocumentPasteEditProvider, dataTransfer: VSDataTransfer): boolean { function isSupportedPasteProvider(provider: DocumentPasteEditProvider, dataTransfer: VSDataTransfer): boolean {
return provider.pasteMimeTypes.some(type => dataTransfer.matches(type)); return Boolean(provider.pasteMimeTypes?.some(type => dataTransfer.matches(type)));
} }

View File

@@ -930,9 +930,10 @@ class MainThreadPasteEditProvider implements languages.DocumentPasteEditProvider
private readonly dataTransfers = new DataTransferFileCache(); private readonly dataTransfers = new DataTransferFileCache();
public readonly copyMimeTypes?: readonly string[]; public readonly copyMimeTypes?: readonly string[];
public readonly pasteMimeTypes: readonly string[]; public readonly pasteMimeTypes?: readonly string[];
readonly prepareDocumentPaste?: languages.DocumentPasteEditProvider['prepareDocumentPaste']; readonly prepareDocumentPaste?: languages.DocumentPasteEditProvider['prepareDocumentPaste'];
readonly provideDocumentPasteEdits?: languages.DocumentPasteEditProvider['provideDocumentPasteEdits'];
constructor( constructor(
private readonly handle: number, private readonly handle: number,
@@ -962,27 +963,29 @@ class MainThreadPasteEditProvider implements languages.DocumentPasteEditProvider
return dataTransferOut; return dataTransferOut;
}; };
} }
}
async provideDocumentPasteEdits(model: ITextModel, selections: Selection[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) { if (metadata.supportsPaste) {
const request = this.dataTransfers.add(dataTransfer); this.provideDocumentPasteEdits = async (model: ITextModel, selections: Selection[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) => {
try { const request = this.dataTransfers.add(dataTransfer);
const dataTransferDto = await typeConvert.DataTransfer.from(dataTransfer); try {
if (token.isCancellationRequested) { const dataTransferDto = await typeConvert.DataTransfer.from(dataTransfer);
return; if (token.isCancellationRequested) {
} return;
}
const result = await this._proxy.$providePasteEdits(this.handle, request.id, model.uri, selections, dataTransferDto, token); const result = await this._proxy.$providePasteEdits(this.handle, request.id, model.uri, selections, dataTransferDto, token);
if (!result) { if (!result) {
return; return;
} }
return { return {
...result, ...result,
additionalEdit: result.additionalEdit ? reviveWorkspaceEditDto(result.additionalEdit, this._uriIdentService, dataId => this.resolveFileData(request.id, dataId)) : undefined, additionalEdit: result.additionalEdit ? reviveWorkspaceEditDto(result.additionalEdit, this._uriIdentService, dataId => this.resolveFileData(request.id, dataId)) : undefined,
};
} finally {
request.dispose();
}
}; };
} finally {
request.dispose();
} }
} }

View File

@@ -1830,8 +1830,9 @@ export type ITypeHierarchyItemDto = Dto<TypeHierarchyItem>;
export interface IPasteEditProviderMetadataDto { export interface IPasteEditProviderMetadataDto {
readonly supportsCopy: boolean; readonly supportsCopy: boolean;
readonly supportsPaste: boolean;
readonly copyMimeTypes?: readonly string[]; readonly copyMimeTypes?: readonly string[];
readonly pasteMimeTypes: readonly string[]; readonly pasteMimeTypes?: readonly string[];
} }
export interface IPasteEditDto { export interface IPasteEditDto {

View File

@@ -530,6 +530,10 @@ class DocumentPasteEditProvider {
} }
async providePasteEdits(requestId: number, resource: URI, ranges: IRange[], dataTransferDto: extHostProtocol.DataTransferDTO, token: CancellationToken): Promise<undefined | extHostProtocol.IPasteEditDto> { async providePasteEdits(requestId: number, resource: URI, ranges: IRange[], dataTransferDto: extHostProtocol.DataTransferDTO, token: CancellationToken): Promise<undefined | extHostProtocol.IPasteEditDto> {
if (!this._provider.provideDocumentPasteEdits) {
return;
}
const doc = this._documents.getDocument(resource); const doc = this._documents.getDocument(resource);
const vscodeRanges = ranges.map(range => typeConvert.Range.to(range)); const vscodeRanges = ranges.map(range => typeConvert.Range.to(range));
@@ -2420,6 +2424,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
this._adapter.set(handle, new AdapterData(new DocumentPasteEditProvider(this._proxy, this._documents, provider, handle, extension), extension)); this._adapter.set(handle, new AdapterData(new DocumentPasteEditProvider(this._proxy, this._documents, provider, handle, extension), extension));
this._proxy.$registerPasteEditProvider(handle, this._transformDocumentSelector(selector, extension), { this._proxy.$registerPasteEditProvider(handle, this._transformDocumentSelector(selector, extension), {
supportsCopy: !!provider.prepareDocumentPaste, supportsCopy: !!provider.prepareDocumentPaste,
supportsPaste: !!provider.provideDocumentPasteEdits,
copyMimeTypes: metadata.copyMimeTypes, copyMimeTypes: metadata.copyMimeTypes,
pasteMimeTypes: metadata.pasteMimeTypes, pasteMimeTypes: metadata.pasteMimeTypes,
}); });

View File

@@ -37,7 +37,7 @@ declare module 'vscode' {
* *
* @return Optional workspace edit that applies the paste. Return undefined to use standard pasting. * @return Optional workspace edit that applies the paste. Return undefined to use standard pasting.
*/ */
provideDocumentPasteEdits(document: TextDocument, ranges: readonly Range[], dataTransfer: DataTransfer, token: CancellationToken): ProviderResult<DocumentPasteEdit>; provideDocumentPasteEdits?(document: TextDocument, ranges: readonly Range[], dataTransfer: DataTransfer, token: CancellationToken): ProviderResult<DocumentPasteEdit>;
} }
/** /**
@@ -98,7 +98,7 @@ declare module 'vscode' {
* Note that {@link DataTransferFile} entries are only created when dropping content from outside the editor, such as * Note that {@link DataTransferFile} entries are only created when dropping content from outside the editor, such as
* from the operating system. * from the operating system.
*/ */
readonly pasteMimeTypes: readonly string[]; readonly pasteMimeTypes?: readonly string[];
} }
namespace languages { namespace languages {