Skip to content

Commit

Permalink
Move await from priority for drop/paste API proposals
Browse files Browse the repository at this point in the history
For #179430, #30066

Switching to use `yieldTo` instead of `priority` to let an extension de-rank itself in the list of edits. `priority` was an arbitrary number while `yieldTo` gives more control over how the ranking takes place
  • Loading branch information
mjbvz committed Aug 8, 2023
1 parent 48aa152 commit 1740397
Show file tree
Hide file tree
Showing 14 changed files with 122 additions and 86 deletions.
17 changes: 3 additions & 14 deletions extensions/ipynb/src/notebookImagePaste.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ enum MimeType {
png = 'image/png',
tiff = 'image/tiff',
webp = 'image/webp',
plain = 'text/plain',
uriList = 'text/uri-list',
}

Expand Down Expand Up @@ -49,8 +50,6 @@ class DropOrPasteEditProvider implements vscode.DocumentPasteEditProvider, vscod

private readonly id = 'insertAttachment';

private readonly defaultPriority = 5;

async provideDocumentPasteEdits(
document: vscode.TextDocument,
_ranges: readonly vscode.Range[],
Expand All @@ -68,7 +67,7 @@ class DropOrPasteEditProvider implements vscode.DocumentPasteEditProvider, vscod
}

const pasteEdit = new vscode.DocumentPasteEdit(insert.insertText, this.id, vscode.l10n.t('Insert Image as Attachment'));
pasteEdit.priority = this.getPastePriority(dataTransfer);
pasteEdit.yieldTo = [{ mimeType: MimeType.plain }];
pasteEdit.additionalEdit = insert.additionalEdit;
return pasteEdit;
}
Expand All @@ -86,22 +85,12 @@ class DropOrPasteEditProvider implements vscode.DocumentPasteEditProvider, vscod

const dropEdit = new vscode.DocumentDropEdit(insert.insertText);
dropEdit.id = this.id;
dropEdit.priority = this.defaultPriority;
dropEdit.yieldTo = [{ mimeType: MimeType.plain }];
dropEdit.additionalEdit = insert.additionalEdit;
dropEdit.label = vscode.l10n.t('Insert Image as Attachment');
return dropEdit;
}

private getPastePriority(dataTransfer: vscode.DataTransfer): number {
if (dataTransfer.get('text/plain')) {
// Deprioritize in favor of normal text content
return -5;
}

// Otherwise boost priority so attachments are preferred
return this.defaultPriority;
}

private async createInsertImageAttachmentEdit(
document: vscode.TextDocument,
dataTransfer: vscode.DataTransfer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ class PasteEditProvider implements vscode.DocumentPasteEditProvider {

private readonly _id = 'insertLink';

private readonly _yieldTo = [
{ mimeType: 'text/plain' },
{ extensionId: 'vscode.ipynb', editId: 'insertAttachment' },
];

async provideDocumentPasteEdits(
document: vscode.TextDocument,
ranges: readonly vscode.Range[],
Expand All @@ -32,15 +37,16 @@ class PasteEditProvider implements vscode.DocumentPasteEditProvider {
if (!urlList) {
return;
}
const pasteUrlSetting = await getPasteUrlAsFormattedLinkSetting(document);

const pasteUrlSetting = getPasteUrlAsFormattedLinkSetting(document);
const pasteEdit = await createEditAddingLinksForUriList(document, ranges, urlList, false, pasteUrlSetting === PasteUrlAsFormattedLink.Smart, token);
if (!pasteEdit) {
return;
}

uriEdit.label = pasteEdit.label;
uriEdit.additionalEdit = pasteEdit.additionalEdits;
uriEdit.priority = this._getPriority(dataTransfer);
uriEdit.yieldTo = this._yieldTo;
return uriEdit;
}

Expand All @@ -61,17 +67,9 @@ class PasteEditProvider implements vscode.DocumentPasteEditProvider {

const pasteEdit = new vscode.DocumentPasteEdit(edit.snippet, this._id, edit.label);
pasteEdit.additionalEdit = edit.additionalEdits;
pasteEdit.priority = this._getPriority(dataTransfer);
pasteEdit.yieldTo = this._yieldTo;
return pasteEdit;
}

private _getPriority(dataTransfer: vscode.DataTransfer): number {
if (dataTransfer.get('text/plain')) {
// Deprioritize in favor of normal text content
return -10;
}
return 0;
}
}

export function registerPasteSupport(selector: vscode.DocumentSelector,) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,60 +5,44 @@

import * as vscode from 'vscode';
import { createEditAddingLinksForUriList, getPasteUrlAsFormattedLinkSetting, PasteUrlAsFormattedLink, validateLink } from './shared';

const textPlainMime = 'text/plain';

class PasteLinkEditProvider implements vscode.DocumentPasteEditProvider {

readonly id = 'insertMarkdownLink';

async provideDocumentPasteEdits(
document: vscode.TextDocument,
ranges: readonly vscode.Range[],
dataTransfer: vscode.DataTransfer,
token: vscode.CancellationToken,
): Promise<vscode.DocumentPasteEdit | undefined> {
const pasteUrlSetting = await getPasteUrlAsFormattedLinkSetting(document);
const pasteUrlSetting = getPasteUrlAsFormattedLinkSetting(document);
if (pasteUrlSetting === PasteUrlAsFormattedLink.Never) {
return;
}

const item = dataTransfer.get('text/plain');
const item = dataTransfer.get(textPlainMime);
const urlList = await item?.asString();

if (urlList === undefined) {
return;
}

if (!validateLink(urlList).isValid) {
if (token.isCancellationRequested || !urlList || !validateLink(urlList).isValid) {
return;
}

const uriEdit = new vscode.DocumentPasteEdit('', this.id, '');
if (!urlList) {
return undefined;
}

const pasteEdit = await createEditAddingLinksForUriList(document, ranges, validateLink(urlList).cleanedUrlList, true, pasteUrlSetting === PasteUrlAsFormattedLink.Smart, token);
if (!pasteEdit) {
return;
}

uriEdit.label = pasteEdit.label;
uriEdit.additionalEdit = pasteEdit.additionalEdits;
uriEdit.priority = this._getPriority(pasteEdit.markdownLink);
return uriEdit;
}

private _getPriority(pasteAsMarkdownLink: boolean): number {
if (!pasteAsMarkdownLink) {
// Deprioritize in favor of default paste
return -10;
}
return 0;
const edit = new vscode.DocumentPasteEdit('', this.id, pasteEdit.label);
edit.additionalEdit = pasteEdit.additionalEdits;
edit.yieldTo = pasteEdit.markdownLink ? undefined : [{ mimeType: textPlainMime }];
return edit;
}
}

export function registerLinkPasteSupport(selector: vscode.DocumentSelector,) {
return vscode.languages.registerDocumentPasteEditProvider(selector, new PasteLinkEditProvider(), {
pasteMimeTypes: [
'text/plain',
]
pasteMimeTypes: [textPlainMime]
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,14 @@ import { Schemes } from '../../util/schemes';


class MarkdownImageDropProvider implements vscode.DocumentDropEditProvider {

private readonly _id = 'insertLink';

private readonly _yieldTo = [
{ mimeType: 'text/plain' },
{ extensionId: 'vscode.ipynb', editId: 'insertAttachment' },
];

async provideDocumentDropEdits(document: vscode.TextDocument, _position: vscode.Position, dataTransfer: vscode.DataTransfer, token: vscode.CancellationToken): Promise<vscode.DocumentDropEdit | undefined> {
const enabled = vscode.workspace.getConfiguration('markdown', document).get('editor.drop.enabled', true);
if (!enabled) {
Expand Down Expand Up @@ -42,6 +48,7 @@ class MarkdownImageDropProvider implements vscode.DocumentDropEditProvider {
const edit = new vscode.DocumentDropEdit(snippet.snippet);
edit.id = this._id;
edit.label = snippet.label;
edit.yieldTo = this._yieldTo;
return edit;
}

Expand All @@ -64,6 +71,7 @@ class MarkdownImageDropProvider implements vscode.DocumentDropEditProvider {
edit.id = this._id;
edit.label = filesEdit.label;
edit.additionalEdit = filesEdit.additionalEdits;
edit.yieldTo = this._yieldTo;
return edit;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export enum PasteUrlAsFormattedLink {
Never = 'never'
}

export async function getPasteUrlAsFormattedLinkSetting(document: vscode.TextDocument): Promise<PasteUrlAsFormattedLink> {
export function getPasteUrlAsFormattedLinkSetting(document: vscode.TextDocument): PasteUrlAsFormattedLink {
return vscode.workspace.getConfiguration('markdown', document).get<PasteUrlAsFormattedLink>('editor.pasteUrlAsFormattedLink.enabled', PasteUrlAsFormattedLink.Smart);
}

Expand Down
11 changes: 9 additions & 2 deletions src/vs/editor/common/languages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -805,7 +805,8 @@ export interface DocumentPasteEdit {
readonly id: string;
readonly label: string;
readonly detail: string;
readonly priority: number;
readonly handledMimeType?: string;
readonly yieldTo?: readonly DropYieldTo[];
insertText: string | { readonly snippet: string };
additionalEdit?: WorkspaceEdit;
}
Expand Down Expand Up @@ -2008,13 +2009,19 @@ export enum ExternalUriOpenerPriority {
Preferred = 3,
}

/**
* @internal
*/
export type DropYieldTo = { readonly editId: string } | { readonly mimeType: string };

/**
* @internal
*/
export interface DocumentOnDropEdit {
readonly id: string;
readonly label: string;
readonly priority: number;
readonly handledMimeType?: string;
readonly yieldTo?: readonly DropYieldTo[];
insertText: string | { readonly snippet: string };
additionalEdit?: WorkspaceEdit;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { Handler, IEditorContribution, PastePayload } from 'vs/editor/common/edi
import { DocumentPasteEdit, DocumentPasteEditProvider } from 'vs/editor/common/languages';
import { ITextModel } from 'vs/editor/common/model';
import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures';
import { createCombinedWorkspaceEdit } from 'vs/editor/contrib/dropOrPasteInto/browser/edit';
import { createCombinedWorkspaceEdit, sortEditsByYieldTo } from 'vs/editor/contrib/dropOrPasteInto/browser/edit';
import { CodeEditorStateFlag, EditorStateCancellationTokenSource } from 'vs/editor/contrib/editorState/browser/editorState';
import { InlineProgressManager } from 'vs/editor/contrib/inlineProgress/browser/inlineProgress';
import { localize } from 'vs/nls';
Expand Down Expand Up @@ -432,18 +432,19 @@ export class CopyPasteController extends Disposable implements IEditorContributi
}

private async getPasteEdits(providers: readonly DocumentPasteEditProvider[], dataTransfer: VSDataTransfer, model: ITextModel, selections: readonly Selection[], token: CancellationToken): Promise<DocumentPasteEdit[]> {
const result = await raceCancellation(
const results = await raceCancellation(
Promise.all(providers.map(provider => {
try {
return provider.provideDocumentPasteEdits?.(model, selections, dataTransfer, token);
} catch (err) {
console.error(err);
return undefined;
}
})).then(coalesce),
})),
token);
result?.sort((a, b) => b.priority - a.priority);
return result ?? [];
const edits = coalesce(results ?? []);
sortEditsByYieldTo(edits);
return edits;
}

private async applyDefaultPasteHandler(dataTransfer: VSDataTransfer, metadata: CopyMetadata | undefined, token: CancellationToken) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ abstract class SimplePasteAndDropProvider implements DocumentOnDropEditProvider,

async provideDocumentPasteEdits(_model: ITextModel, _ranges: readonly IRange[], dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined> {
const edit = await this.getEdit(dataTransfer, token);
return edit ? { id: this.id, insertText: edit.insertText, label: edit.label, detail: edit.detail, priority: edit.priority } : undefined;
return edit ? { id: this.id, insertText: edit.insertText, label: edit.label, detail: edit.detail, handledMimeType: edit.handledMimeType, yieldTo: edit.yieldTo } : undefined;
}

async provideDocumentOnDropEdits(_model: ITextModel, _position: IPosition, dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentOnDropEdit | undefined> {
const edit = await this.getEdit(dataTransfer, token);
return edit ? { id: this.id, insertText: edit.insertText, label: edit.label, priority: edit.priority } : undefined;
return edit ? { id: this.id, insertText: edit.insertText, label: edit.label, handledMimeType: edit.handledMimeType, yieldTo: edit.yieldTo } : undefined;
}

protected abstract getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined>;
Expand All @@ -46,7 +46,7 @@ class DefaultTextProvider extends SimplePasteAndDropProvider {
readonly dropMimeTypes = [Mimes.text];
readonly pasteMimeTypes = [Mimes.text];

protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, _token: CancellationToken) {
protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, _token: CancellationToken): Promise<DocumentPasteEdit | undefined> {
const textEntry = dataTransfer.get(Mimes.text);
if (!textEntry) {
return;
Expand All @@ -61,7 +61,7 @@ class DefaultTextProvider extends SimplePasteAndDropProvider {
const insertText = await textEntry.asString();
return {
id: this.id,
priority: 0,
handledMimeType: Mimes.text,
label: localize('text.label', "Insert Plain Text"),
detail: builtInLabel,
insertText
Expand All @@ -75,7 +75,7 @@ class PathProvider extends SimplePasteAndDropProvider {
readonly dropMimeTypes = [Mimes.uriList];
readonly pasteMimeTypes = [Mimes.uriList];

protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) {
protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined> {
const entries = await extractUriList(dataTransfer);
if (!entries.length || token.isCancellationRequested) {
return;
Expand Down Expand Up @@ -108,7 +108,7 @@ class PathProvider extends SimplePasteAndDropProvider {

return {
id: this.id,
priority: 0,
handledMimeType: Mimes.uriList,
insertText,
label,
detail: builtInLabel,
Expand All @@ -128,7 +128,7 @@ class RelativePathProvider extends SimplePasteAndDropProvider {
super();
}

protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken) {
protected async getEdit(dataTransfer: IReadonlyVSDataTransfer, token: CancellationToken): Promise<DocumentPasteEdit | undefined> {
const entries = await extractUriList(dataTransfer);
if (!entries.length || token.isCancellationRequested) {
return;
Expand All @@ -145,7 +145,7 @@ class RelativePathProvider extends SimplePasteAndDropProvider {

return {
id: this.id,
priority: 0,
handledMimeType: Mimes.uriList,
insertText: relativeUris.join(' '),
label: entries.length > 1
? localize('defaultDropProvider.uriList.relativePaths', "Insert Relative Paths")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { RawContextKey } from 'vs/platform/contextkey/common/contextkey';
import { LocalSelectionTransfer } from 'vs/platform/dnd/browser/dnd';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { PostEditWidgetManager } from './postEditWidget';
import { sortEditsByYieldTo as sortEditsByYieldTo } from './edit';

export const changeDropTypeCommandId = 'editor.changeDropType';

Expand Down Expand Up @@ -128,7 +129,7 @@ export class DropIntoEditorController extends Disposable implements IEditorContr
return provider.provideDocumentOnDropEdits(model, position, dataTransfer, tokenSource.token);
})), tokenSource.token);
const edits = coalesce(results ?? []);
edits.sort((a, b) => b.priority - a.priority);
sortEditsByYieldTo(edits);
return edits;
}

Expand Down
25 changes: 24 additions & 1 deletion src/vs/editor/contrib/dropOrPasteInto/browser/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { URI } from 'vs/base/common/uri';
import { ResourceTextEdit } from 'vs/editor/browser/services/bulkEditService';
import { WorkspaceEdit } from 'vs/editor/common/languages';
import { DropYieldTo, WorkspaceEdit } from 'vs/editor/common/languages';
import { Range } from 'vs/editor/common/core/range';

export interface DropOrPasteEdit {
Expand All @@ -27,3 +27,26 @@ export function createCombinedWorkspaceEdit(uri: URI, ranges: readonly Range[],
]
};
}

export function sortEditsByYieldTo<T extends {
readonly id: string;
readonly handledMimeType?: string;
readonly yieldTo?: readonly DropYieldTo[];
}>(edits: T[]): void {
function yieldsTo(yTo: DropYieldTo, other: T): boolean {
return ('editId' in yTo && yTo.editId === other.id)
|| ('mimeType' in yTo && yTo.mimeType === other.handledMimeType);
}

edits.sort((a, b) => {
if (a.yieldTo?.some(yTo => yieldsTo(yTo, b))) {
return 1;
}

if (b.yieldTo?.some(yTo => yieldsTo(yTo, a))) {
return -1;
}

return 0;
});
}
Loading

0 comments on commit 1740397

Please sign in to comment.