Skip to content

Commit 9b6b547

Browse files
mjbvzmjbvz
authored
Fix range when copying empty selection (microsoft#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
1 parent 068cbf3 commit 9b6b547

File tree

9 files changed

+241
-30
lines changed

9 files changed

+241
-30
lines changed

‎extensions/vscode-api-tests/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"customEditorMove",
1111
"diffCommand",
1212
"documentFiltersExclusive",
13+
"documentPaste",
1314
"editorInsets",
1415
"extensionRuntime",
1516
"extensionsAny",

‎extensions/vscode-api-tests/src/singlefolder-tests/documentPaste.test.ts

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
import * as assert from 'assert';
7+
import * as vscode from 'vscode';
8+
import { assertNoRpc, createRandomFile, usingDisposables } from '../utils';
9+
10+
const textPlain = 'text/plain';
11+
12+
(vscode.env.uiKind === vscode.UIKind.Web ? suite.skip : suite)('vscode API - Copy Paste', () => {
13+
14+
teardown(assertNoRpc);
15+
16+
test('Copy should be able to overwrite text/plain', usingDisposables(async (disposables) => {
17+
const file = await createRandomFile('$abcde@');
18+
const doc = await vscode.workspace.openTextDocument(file);
19+
20+
const editor = await vscode.window.showTextDocument(doc);
21+
editor.selections = [new vscode.Selection(0, 1, 0, 6)];
22+
23+
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
24+
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
25+
const existing = dataTransfer.get(textPlain);
26+
if (existing) {
27+
const str = await existing.asString();
28+
const reversed = reverseString(str);
29+
dataTransfer.set(textPlain, new vscode.DataTransferItem(reversed));
30+
}
31+
}
32+
}, { copyMimeTypes: [textPlain] }));
33+
34+
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
35+
const newDocContent = getNextDocumentText(disposables, doc);
36+
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
37+
assert.strictEqual(await newDocContent, '$edcba@');
38+
}));
39+
40+
test('Copy with empty selection should copy entire line', usingDisposables(async (disposables) => {
41+
const file = await createRandomFile('abc\ndef');
42+
const doc = await vscode.workspace.openTextDocument(file);
43+
await vscode.window.showTextDocument(doc);
44+
45+
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
46+
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
47+
const existing = dataTransfer.get(textPlain);
48+
if (existing) {
49+
const str = await existing.asString();
50+
// text/plain includes the trailing new line in this case
51+
// On windows, this will always be `\r\n` even if the document uses `\n`
52+
const eol = str.match(/\r?\n$/);
53+
const reversed = reverseString(str.slice(0, -eol![0].length));
54+
dataTransfer.set(textPlain, new vscode.DataTransferItem(reversed + '\n'));
55+
}
56+
}
57+
}, { copyMimeTypes: [textPlain] }));
58+
59+
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
60+
const newDocContent = getNextDocumentText(disposables, doc);
61+
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
62+
assert.strictEqual(await newDocContent, `cba\nabc\ndef`);
63+
}));
64+
65+
test('Copy with multiple selections should get all selections', usingDisposables(async (disposables) => {
66+
const file = await createRandomFile('111\n222\n333');
67+
const doc = await vscode.workspace.openTextDocument(file);
68+
const editor = await vscode.window.showTextDocument(doc);
69+
70+
editor.selections = [
71+
new vscode.Selection(0, 0, 0, 3),
72+
new vscode.Selection(2, 0, 2, 3),
73+
];
74+
75+
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
76+
async prepareDocumentPaste(document: vscode.TextDocument, ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
77+
const existing = dataTransfer.get(textPlain);
78+
if (existing) {
79+
const selections = ranges.map(range => document.getText(range));
80+
dataTransfer.set(textPlain, new vscode.DataTransferItem(`(${ranges.length})${selections.join(' ')}`));
81+
}
82+
}
83+
}, { copyMimeTypes: [textPlain] }));
84+
85+
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
86+
editor.selections = [new vscode.Selection(0, 0, 0, 0)];
87+
const newDocContent = getNextDocumentText(disposables, doc);
88+
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
89+
90+
assert.strictEqual(await newDocContent, `(2)111 333111\n222\n333`);
91+
}));
92+
93+
test('Earlier invoked copy providers should win when writing values', usingDisposables(async (disposables) => {
94+
const file = await createRandomFile('abc\ndef');
95+
const doc = await vscode.workspace.openTextDocument(file);
96+
97+
const editor = await vscode.window.showTextDocument(doc);
98+
editor.selections = [new vscode.Selection(0, 0, 0, 3)];
99+
100+
const callOrder: string[] = [];
101+
const a_id = 'a';
102+
const b_id = 'b';
103+
104+
let providerAResolve: () => void;
105+
const providerAFinished = new Promise<void>(resolve => providerAResolve = resolve);
106+
107+
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
108+
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
109+
callOrder.push(a_id);
110+
dataTransfer.set(textPlain, new vscode.DataTransferItem('a'));
111+
providerAResolve();
112+
}
113+
}, { copyMimeTypes: [textPlain] }));
114+
115+
// Later registered providers will be called first
116+
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
117+
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
118+
callOrder.push(b_id);
119+
120+
// Wait for the first provider to finish even though we were called first.
121+
// This tests that resulting order does not depend on the order the providers
122+
// return in.
123+
await providerAFinished;
124+
125+
dataTransfer.set(textPlain, new vscode.DataTransferItem('b'));
126+
}
127+
}, { copyMimeTypes: [textPlain] }));
128+
129+
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
130+
const newDocContent = getNextDocumentText(disposables, doc);
131+
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
132+
assert.strictEqual(await newDocContent, 'b\ndef');
133+
134+
// Confirm provider call order is what we expected
135+
assert.deepStrictEqual(callOrder, [b_id, a_id]);
136+
}));
137+
138+
test('Copy providers should not be able to effect the data transfer of another', usingDisposables(async (disposables) => {
139+
const file = await createRandomFile('abc\ndef');
140+
const doc = await vscode.workspace.openTextDocument(file);
141+
142+
const editor = await vscode.window.showTextDocument(doc);
143+
editor.selections = [new vscode.Selection(0, 0, 0, 3)];
144+
145+
146+
let providerAResolve: () => void;
147+
const providerAFinished = new Promise<void>(resolve => providerAResolve = resolve);
148+
149+
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
150+
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
151+
dataTransfer.set(textPlain, new vscode.DataTransferItem('xyz'));
152+
providerAResolve();
153+
}
154+
}, { copyMimeTypes: [textPlain] }));
155+
156+
disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
157+
async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
158+
159+
// Wait for the first provider to finish
160+
await providerAFinished;
161+
162+
// We we access the data transfer here, we should not see changes made by the first provider
163+
const entry = dataTransfer.get(textPlain);
164+
const str = await entry!.asString();
165+
dataTransfer.set(textPlain, new vscode.DataTransferItem(reverseString(str)));
166+
}
167+
}, { copyMimeTypes: [textPlain] }));
168+
169+
await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
170+
const newDocContent = getNextDocumentText(disposables, doc);
171+
await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
172+
assert.strictEqual(await newDocContent, 'cba\ndef');
173+
174+
}));
175+
});
176+
177+
function reverseString(str: string) {
178+
return str.split("").reverse().join("");
179+
}
180+
181+
function getNextDocumentText(disposables: vscode.Disposable[], doc: vscode.TextDocument): Promise<string> {
182+
return new Promise<string>(resolve => {
183+
disposables.push(vscode.workspace.onDidChangeTextDocument(e => {
184+
if (e.document === doc) {
185+
resolve(doc.getText());
186+
}
187+
}));
188+
});
189+
}
190+

‎extensions/vscode-api-tests/src/utils.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,17 @@ export function disposeAll(disposables: vscode.Disposable[]) {
6262
vscode.Disposable.from(...disposables).dispose();
6363
}
6464

65+
export function usingDisposables<R>(fn: (this: Mocha.Context, store: vscode.Disposable[]) => Promise<R>) {
66+
return async function (this: Mocha.Context): Promise<R> {
67+
const disposables: vscode.Disposable[] = [];
68+
try {
69+
return await fn.call(this, disposables);
70+
} finally {
71+
disposeAll(disposables);
72+
}
73+
};
74+
}
75+
6576
export function delay(ms: number) {
6677
return new Promise(resolve => setTimeout(resolve, ms));
6778
}

‎src/vs/editor/common/languages.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -799,11 +799,11 @@ export interface DocumentPasteEditProvider {
799799
readonly id?: string;
800800

801801
readonly copyMimeTypes?: readonly string[];
802-
readonly pasteMimeTypes: readonly string[];
802+
readonly pasteMimeTypes?: readonly string[];
803803

804804
prepareDocumentPaste?(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<undefined | IReadonlyVSDataTransfer>;
805805

806-
provideDocumentPasteEdits(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
806+
provideDocumentPasteEdits?(model: model.ITextModel, ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
807807
}
808808

809809
/**

‎src/vs/editor/contrib/dropOrPasteInto/browser/copyPasteController.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
133133
return;
134134
}
135135

136-
ranges = ranges.map(range => new Range(range.startLineNumber, 0, range.startLineNumber, model.getLineLength(range.startLineNumber)));
136+
ranges = [new Range(ranges[0].startLineNumber, 1, ranges[0].startLineNumber, 1 + model.getLineLength(ranges[0].startLineNumber))];
137137
}
138138

139139
const toCopy = this._editor._getViewModel()?.getPlainTextToCopy(selections, enableEmptySelectionClipboard, platform.isWindows);
@@ -219,7 +219,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
219219

220220
const allProviders = this._languageFeaturesService.documentPasteEditProvider
221221
.ordered(model)
222-
.filter(provider => provider.pasteMimeTypes.some(type => matchesMimeType(type, allPotentialMimeTypes)));
222+
.filter(provider => provider.pasteMimeTypes?.some(type => matchesMimeType(type, allPotentialMimeTypes)));
223223
if (!allProviders.length) {
224224
return;
225225
}
@@ -253,7 +253,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
253253
}
254254

255255
// Filter out any providers the don't match the full data transfer we will send them.
256-
const supportedProviders = allProviders.filter(provider => isSupportedProvider(provider, dataTransfer));
256+
const supportedProviders = allProviders.filter(provider => isSupportedPasteProvider(provider, dataTransfer));
257257
if (!supportedProviders.length
258258
|| (supportedProviders.length === 1 && supportedProviders[0].id === 'text') // Only our default text provider is active
259259
) {
@@ -300,7 +300,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
300300
}
301301

302302
// Filter out any providers the don't match the full data transfer we will send them.
303-
const supportedProviders = allProviders.filter(provider => isSupportedProvider(provider, dataTransfer));
303+
const supportedProviders = allProviders.filter(provider => isSupportedPasteProvider(provider, dataTransfer));
304304

305305
const providerEdits = await this.getPasteEdits(supportedProviders, dataTransfer, model, selections, tokenSource.token);
306306
if (tokenSource.token.isCancellationRequested) {
@@ -392,7 +392,7 @@ export class CopyPasteController extends Disposable implements IEditorContributi
392392
private async getPasteEdits(providers: readonly DocumentPasteEditProvider[], dataTransfer: VSDataTransfer, model: ITextModel, selections: readonly Selection[], token: CancellationToken): Promise<DocumentPasteEdit[]> {
393393
const result = await raceCancellation(
394394
Promise.all(
395-
providers.map(provider => provider.provideDocumentPasteEdits(model, selections, dataTransfer, token))
395+
providers.map(provider => provider.provideDocumentPasteEdits?.(model, selections, dataTransfer, token))
396396
).then(coalesce),
397397
token);
398398
result?.sort((a, b) => b.priority - a.priority);
@@ -420,6 +420,6 @@ export class CopyPasteController extends Disposable implements IEditorContributi
420420
}
421421
}
422422

423-
function isSupportedProvider(provider: DocumentPasteEditProvider, dataTransfer: VSDataTransfer): boolean {
424-
return provider.pasteMimeTypes.some(type => dataTransfer.matches(type));
423+
function isSupportedPasteProvider(provider: DocumentPasteEditProvider, dataTransfer: VSDataTransfer): boolean {
424+
return Boolean(provider.pasteMimeTypes?.some(type => dataTransfer.matches(type)));
425425
}

‎src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -930,9 +930,10 @@ class MainThreadPasteEditProvider implements languages.DocumentPasteEditProvider
930930
private readonly dataTransfers = new DataTransferFileCache();
931931

932932
public readonly copyMimeTypes?: readonly string[];
933-
public readonly pasteMimeTypes: readonly string[];
933+
public readonly pasteMimeTypes?: readonly string[];
934934

935935
readonly prepareDocumentPaste?: languages.DocumentPasteEditProvider['prepareDocumentPaste'];
936+
readonly provideDocumentPasteEdits?: languages.DocumentPasteEditProvider['provideDocumentPasteEdits'];
936937

937938
constructor(
938939
private readonly handle: number,
@@ -962,27 +963,29 @@ class MainThreadPasteEditProvider implements languages.DocumentPasteEditProvider
962963
return dataTransferOut;
963964
};
964965
}
965-
}
966966

967-
async provideDocumentPasteEdits(model: ITextModel, selections: Selection[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) {
968-
const request = this.dataTransfers.add(dataTransfer);
969-
try {
970-
const dataTransferDto = await typeConvert.DataTransfer.from(dataTransfer);
971-
if (token.isCancellationRequested) {
972-
return;
973-
}
967+
if (metadata.supportsPaste) {
968+
this.provideDocumentPasteEdits = async (model: ITextModel, selections: Selection[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) => {
969+
const request = this.dataTransfers.add(dataTransfer);
970+
try {
971+
const dataTransferDto = await typeConvert.DataTransfer.from(dataTransfer);
972+
if (token.isCancellationRequested) {
973+
return;
974+
}
974975

975-
const result = await this._proxy.$providePasteEdits(this.handle, request.id, model.uri, selections, dataTransferDto, token);
976-
if (!result) {
977-
return;
978-
}
976+
const result = await this._proxy.$providePasteEdits(this.handle, request.id, model.uri, selections, dataTransferDto, token);
977+
if (!result) {
978+
return;
979+
}
979980

980-
return {
981-
...result,
982-
additionalEdit: result.additionalEdit ? reviveWorkspaceEditDto(result.additionalEdit, this._uriIdentService, dataId => this.resolveFileData(request.id, dataId)) : undefined,
981+
return {
982+
...result,
983+
additionalEdit: result.additionalEdit ? reviveWorkspaceEditDto(result.additionalEdit, this._uriIdentService, dataId => this.resolveFileData(request.id, dataId)) : undefined,
984+
};
985+
} finally {
986+
request.dispose();
987+
}
983988
};
984-
} finally {
985-
request.dispose();
986989
}
987990
}
988991

‎src/vs/workbench/api/common/extHost.protocol.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1830,8 +1830,9 @@ export type ITypeHierarchyItemDto = Dto<TypeHierarchyItem>;
18301830

18311831
export interface IPasteEditProviderMetadataDto {
18321832
readonly supportsCopy: boolean;
1833+
readonly supportsPaste: boolean;
18331834
readonly copyMimeTypes?: readonly string[];
1834-
readonly pasteMimeTypes: readonly string[];
1835+
readonly pasteMimeTypes?: readonly string[];
18351836
}
18361837

18371838
export interface IPasteEditDto {

‎src/vs/workbench/api/common/extHostLanguageFeatures.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,10 @@ class DocumentPasteEditProvider {
530530
}
531531

532532
async providePasteEdits(requestId: number, resource: URI, ranges: IRange[], dataTransferDto: extHostProtocol.DataTransferDTO, token: CancellationToken): Promise<undefined | extHostProtocol.IPasteEditDto> {
533+
if (!this._provider.provideDocumentPasteEdits) {
534+
return;
535+
}
536+
533537
const doc = this._documents.getDocument(resource);
534538
const vscodeRanges = ranges.map(range => typeConvert.Range.to(range));
535539

@@ -2420,6 +2424,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF
24202424
this._adapter.set(handle, new AdapterData(new DocumentPasteEditProvider(this._proxy, this._documents, provider, handle, extension), extension));
24212425
this._proxy.$registerPasteEditProvider(handle, this._transformDocumentSelector(selector, extension), {
24222426
supportsCopy: !!provider.prepareDocumentPaste,
2427+
supportsPaste: !!provider.provideDocumentPasteEdits,
24232428
copyMimeTypes: metadata.copyMimeTypes,
24242429
pasteMimeTypes: metadata.pasteMimeTypes,
24252430
});

‎src/vscode-dts/vscode.proposed.documentPaste.d.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ declare module 'vscode' {
3737
*
3838
* @return Optional workspace edit that applies the paste. Return undefined to use standard pasting.
3939
*/
40-
provideDocumentPasteEdits(document: TextDocument, ranges: readonly Range[], dataTransfer: DataTransfer, token: CancellationToken): ProviderResult<DocumentPasteEdit>;
40+
provideDocumentPasteEdits?(document: TextDocument, ranges: readonly Range[], dataTransfer: DataTransfer, token: CancellationToken): ProviderResult<DocumentPasteEdit>;
4141
}
4242

4343
/**
@@ -98,7 +98,7 @@ declare module 'vscode' {
9898
* Note that {@link DataTransferFile} entries are only created when dropping content from outside the editor, such as
9999
* from the operating system.
100100
*/
101-
readonly pasteMimeTypes: readonly string[];
101+
readonly pasteMimeTypes?: readonly string[];
102102
}
103103

104104
namespace languages {

0 commit comments

Comments
 (0)