The Wayback Machine - https://web.archive.org/web/20220407134030/https://github.com/microsoft/vscode/issues/30066
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

Add events for copy/paste to allow language extensions to bring using/import statements along #30066

Open
DanTup opened this issue Jul 3, 2017 · 25 comments
Assignees
Labels
api editor-code-actions feature-request
Milestone

Comments

@DanTup
Copy link
Contributor

@DanTup DanTup commented Jul 3, 2017

Apologies if this already exists, I can't seem to find anything about it (I expected it'd be listed in complex commands if anywhere).

The language service I use for Dart is getting a new feature that will allow me to send a command to get data about imports/using statements that apply to a subset of code. It's designed to be run when a user copies a chunk of code to the clipboard. When pasting, this data can be exchanged for a bunch of edits that would add the equivalent imports in the new file to avoid the user ending up having to fire a bunch of "import xxx" code actions after pasting.

In order to do this I need to know when the user has copied something into their clipboard (including the file and offset/range) and also again after they have pasted (just the file) so I can add some additional edits.

@DanTup
Copy link
Contributor Author

@DanTup DanTup commented Jul 6, 2017

It would actually work out better if we could also attach additional data into the clipboard during copy (to avoid there being a mismatch between the import data we stashed and the clipboard, which might have been replaced externally since) and get it back during paste; but I don't know enough about how clipboards work to know how feasible that is!

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Sep 18, 2020

Having copy/paste bring along imports is a good motivating use case for designing this api. Here is some of what makes it interesting:

  • Multiple different extensions would likely want to offer this functionality.
  • Implementing it requires state management. Store state on copy. Retrieve it on paste.
  • An extension can not precompute the text edits for this and cannot apply them synchronously.
  • On paste, an extension many want to make text edits outside of the paste text (such as adding imports at the top of a file).
  • On paste, an extension may want to make text edits to the pasted text. For example, consider copying a block of JS that uses alias imports and then pasting that into a file that imports the same file as a namespace. We'd want to convert the alias imports in the pasted text to use namespace imports.

Some goals:

  • Simple VS Code setting to control paste behavior
  • Avoid text flickers (where you paste and then the text you just pasted is rewritten a 200ms later)
  • Still let users do a basic paste (without triggering any actions)
  • Don't let misbehaving extensions block paste (have some sort of time box on paste)

Sketches

(note that these are not API proposals, just quick ideas on how this could work)

Source Actions

Try to reuse source actions for implement an editor.codeActionsOnPaste setting. For example, the setting:

"editor.codeActionsOnPaste": [
    "source.organizeImports"
]

Would run organize imports source action whenever the user pastes.

I don't see this as being enough on its own, except for super simple use cases such as organize imports. We need some way for extensions to store state from a copy and use that to produce a set of text edits on paste. Source actions don't provide enough infrastructure on their own

Extend clipboard api

Let extensions be notified of clipboard copy and paste events. Let extensions store metadata on the clipboard.

vscode.env.clipboard.onCopy(async entry => {
     const metadata = await getMetadataForSelection();

     entry.addMetadata('myExtension', metadata);
});


vscode.env.clipboard.onPaste(async entry => {
     const metadata = entry.getMetadata('myExtension';

     if (metadata) {
            // convert metadata into text edits and apply these
     }
});

This leaves the implementation largely up to individual extensions. There are a few big open questions here too:

  • Extensions are driving the behavior (not VS Code)
  • No unified setting to control this behavior (each extension would likely bring along its own)

Paste Action Provider

Define a contract that extensions can implement to hook into copy/paste. This basically just adds a contract on top of the clipboard api sketch above:

class AddImportsOnPasteProvider implements vscode.PasteActionProvider {
   
    async providePasteActions(document: vsocode.TextDocument, copiedRange: vscode.Range): Promise<vscode.PasteAction[]> {

         // Get metadata about copied text
         const metadata = await getMetadataForSelection(document, copiedRange);

         // Return command that VS Code applies on paste
         return [{
             id: 'typescript.moveImports',
             command: <vscode.Command>{
                 title: 'Move Imports',
                 command: 'typescript.doMoveImports',
                 args: [metadata]
             }
         }];
    }
}

Instead of using commands, we could have have an applyPaste method on vscode.PasteActionProvider that would be invoked a paste.

@jrieken
Copy link
Member

@jrieken jrieken commented Sep 18, 2020

So, the extended proposed is somewhat analogue to format on save which happens automatically and the onWillSave-event for less uniform cases, right?

A critical piece in this would be those clipboard events. E.g. when copying from a website and pasting into the editor, then I don't believe that we have a chance to know that copy has happened. Not even sure if we know that inside the editor, e.g when copy/pasting between two files...

@DanTup
Copy link
Contributor Author

@DanTup DanTup commented Sep 18, 2020

If I understand correctly, in the "Paste Action Provider" example it looks like the metadata is collected during the paste operation:

// Get metadata about copied text
const metadata = await getMetadataForSelection(document, copiedRange);

I think that would fall down if a user copies text, the modifies it/deletes it/hits save (which formats). I think the metadata would have to be collected during the copy to be reliable (so the provider would probably need to have hooks for both copy and paste?).

Is this something that could also be supported for LSP? (if so, it may be worth considering how the API would translate to that).

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Sep 18, 2020

@jrieken I originally thought this issue would be as simple as adding an editor.codeActionsOnPaste setting, but it turns out that that would not handle most of the interesting API use cases. For a use case such as moving imports around with copy/paste, the key part is that the operation is stateful. We can either leave managing this state up to extensions or go with a more provider based approach that allows VS Code to manage this state.

We can probably implement these concepts on desktop but I'm not sure about web. I think we should be able to use the copy and paste events, but these may trigger permissions dialogs.

@DanTup The extension would need to save off the metadata in such a way that, on paste, the extension can handle any changes that happened since the copy

A provider based approach is a better fit for the LSP. The goal is to implement it in VS Code first, then add it to the LSP once it has been stabilized

@mjbvz mjbvz removed this from the Backlog milestone Sep 18, 2020
@mjbvz mjbvz added this to the September 2020 milestone Sep 18, 2020
@DanTup
Copy link
Contributor Author

@DanTup DanTup commented Sep 19, 2020

@DanTup The extension would need to save off the metadata in such a way that, on paste, the extension can handle any changes that happened since the copy

Yep, that's what I have in mind - but it needs the Copy event to do this too (the provider example above doesn't seem sufficient). My expectation is that when you Copy, the extension would collect a list of all imports required for that code, then when you paste, it would generate the correct code to import any that are not already imported in the new file (and account for relative paths). That shouldn't be affected by any changes happening in between.

@testforstephen
Copy link

@testforstephen testforstephen commented Sep 21, 2020

Another use case for copy/paste is auto-escape string on paste. For example, pasting some text to a Java string literal, auto escape the characters such as \," etc.

The current PasteActionProvider seems to only consider copy/paste between editors. if copying a piece of content from outside (for example, copy a file path from File Explorer), is there a way to make the extension participate in the paste actions?

@jrieken
Copy link
Member

@jrieken jrieken commented Sep 21, 2020

We can probably implement these concepts on desktop but I'm not sure about web. I think we should be able to use the copy and paste events, but these may trigger permissions dialogs.

Didn't know about those events. Seems to be working for desktop and web. Also, us moving to a node-free-renderer means that it should really work with web. It seems tho that they don't work with navigator.clipboard (which we use to implement the clipboard API) but only with copy/paste that was initiated from a user gesture. Needs maybe a little investigation but looks promising

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Sep 22, 2020

Definitely some limitations with copy and paste events but here's what I've come up with for the implementation:

// @ts-check

/**
 * Use this to save off a async resolved clipboard entry.
 *
 * In the example we resolve this eagerly but you could also resolve it lazily instead
 * 
 * @type {{ handle: string, value?: string } | undefined}
 */
let clipboardItem;

document.addEventListener('copy', e => {
    const handle = '' + Date.now();

    // Save off a handle pointing to data that VS Code maintains.
    e.clipboardData.setData('x-vscode/id', handle);
    clipboardItem = { handle: handle }

    // Simulate the extension resolving the clipboard data asynchronously  
    setTimeout(() => {
        // Make sure we are still on the same copy
        if (clipboardItem?.handle === handle) {
            clipboardItem.value = 'my custom value'
        }
    }, 500);

    // Call prevent default to prevent out new clipboard data from being overwritten (is this really required?)
    e.preventDefault();

    // And then fill in raw text again since we prevented default
    e.clipboardData.setData('text/plain', document.getSelection()?.toString() ?? '');
});

document.addEventListener('paste', e => {
    // Check to see if the copy for this paste came from VS Code
    const id = e.clipboardData.getData('x-vscode/id');

    // If it did, make sure our clipboard data still belongs to the copy that generated it.
    if (id === clipboardItem?.handle) {
        const value = clipboardItem.value;

        // Handle the case where the clipboard has not been resolved yet
        if (typeof value === 'undefined') {
            // Reset
            clipboardItem = undefined;

            // Note that we could wait on a Promise or do something else here...
        } else {

            // Our clipboard item has resolved and is still revevant!
            e.preventDefault();

            // Modify the document based on it
            /** @type {HTMLTextAreaElement | undefined} */
            const element = e.target;

            const selectionStart = element.selectionStart || 0;
            const selectionEnd = element.selectionEnd || 0;

            element.value = `${element.value.substring(0, selectionStart)}${value}${element.value.substring(selectionEnd, element.value.length)}`;
            element.selectionStart = selectionStart + value.length;
            element.selectionEnd = element.selectionStart;
        }
    }
})

This shows that we should be able to :

  • Save arbitrary, asynchronously resolved data to the clipboard
  • Restore that data on paste
  • Prevent the default copy/paste if needed

@jrieken
Copy link
Member

@jrieken jrieken commented Sep 22, 2020

What concerns me is that this flow only works if you copy from within the editor. No treatment when copying from external sources like stackoverflow - which is likely in more need of post-paste-cleanup.

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Sep 22, 2020

Yes this was just an experiment to see if I could implement what this api will require.

For the next step, I'm going to try implementing a VS Code API that is closer to this:

interface CopyPasteActionProvider<T = unknown> {

	/**
	 * Optional method invoked after the user copies some text in a file.
	 * 
	 * @param document Document where the copy took place.
	 * @param selection Selection being copied in the `document`
	 * @param clipboard Information about the clipboard state after the copy.
	 * 
	 * @return Optional metadata that is passed to `onWillPaste`.
	 */
	onDidCopy?(
		document: vscode.TextDocument,
		selection: vscode.Selection,
		clipboard: { readonly text: string },
	): Promise<{ readonly data: T } | undefined>;

	/**
	 * Invoked before the user pastes into a document.
	 * 
	 * @param document Document being pasted into
	 * @param selection Current selection in the document.
	 * @param clipboard Information about the clipboard state. This may contain the metadata from `onDidCopy`.
	 * 
	 * @return Optional workspace edit that applies the paste (TODO: right now always requires implementer to also implement basic paste)
	 */
	onWillPaste(
		document: vscode.TextDocument,
		selection: vscode.Selection,
		clipboard: {
			readonly text: string;
			readonly data?: T;
		},
	): Promise<WorkspaceEdit | undefined>;
}

interface CopyPasteActionProviderMetadata {
	/**
	 * Identifies the type of paste action being returned, such as `moveImports`. (maybe this should just be a simple string)
	 */
	readonly kind: CodeActionKind; 
}

function registerCopyPasteActionProvider(
	selector: vscode.DocumentSelector,
	provider: CopyPasteActionProvider,
	metadata: CopyPasteActionProviderMetadata
): Disposable;

@mjbvz mjbvz removed this from the November 2020 milestone Dec 1, 2020
@mjbvz mjbvz added this to the December/January 2021 milestone Dec 1, 2020
@mjbvz mjbvz removed this from the January 2021 milestone Jan 23, 2021
@mjbvz mjbvz added this to the February 2021 milestone Jan 23, 2021
@dbaeumer
Copy link
Member

@dbaeumer dbaeumer commented Feb 3, 2021

This came up lately in LSP as well and from my experience implementing this for languages in the past I think there are more things to consider

  • capturing the imports on copy is not enough. Besides the fact that you might copy from another source the imports might not make any sense in the new paste location.
  • when pasting (even if the imports are visible in the paste location) that pasted code might not be correct in that location due to scoping issues (for example on copy you see a type Foo which is imported) but in the pasted location Foo is a local type.

So I think we need to define whether copy / paste should try to be semantic preserving to the pasted code or whether we want to make the code work in the pasted location. I personally tend to implement the second approach.

Implementation wise having a onWillPaste puts quite some implementation complexity onto the server. To really answer the questions of fixing imports and making the code work the server needs to create a working copy of the file, paste in the code, build an AST and run a type binding phase. All of this shouldn't have any side effect onto other code (e.g. no diagnostics should be created in another file).

I see the point of flickering when we first paste and then do the fixing but it might not be so bad since:

  • I don't expect that a paste is semantic preserving (e.g. qualifies Foo in the case Foo is also visible in the local scope)
  • we already do this for format on paste
  • the imports are usually not visible (in the editor) anyways. We do the same for example with additional text edits in the completion item.

So for me the captured imports on copy would only act as a hint in case the pasted code result in errors in that location.

@alexdima
Copy link
Member

@alexdima alexdima commented Feb 3, 2021

Strictly from the editor's point of view, making copy or paste async (in order to allow for async participants) might not be feasible. In the web, we get copy/paste browser events and we have access to the clipboard data transfer object only in the event handlers themselves.

Making things async would mean we would always have to go through navigator.clipboard which prompts to the user (in the editor this is only used when right click > Paste is used, which is in my experience pretty rare compared to ctrl+v). When using ctrl+c/ctrl+x/ctrl+v, the editor does not prompt for permissions. Also, navigator.clipboard is not really supported in all browsers entirely, for example it doesn't work to read from the clipboard in Firefox (that's why we don't have right click > Paste in FF).

That being said, we already store some metadata when copying to the clipboard. For example, we remember if the copy happened with multiple cursors or not in order to behave differently when pasting. We use a special clipboard mimetype vscode-editor-data (i.e. different than plaintext and text/html). When pasting, we can then read this metadata again. As part of this metadata, we could potentially store the URI and the range where the copy or cut event occurred in case this might be useful when pasting.

Also, making pasting async might become problematic w.r.t. timing and user expectation. What should happen when a new key press occurs after a paste and onWillPaste still hasn't returned. Should there be a time limit enforced? What if the time limit is reached? Is it possible to manually execute the "post paste massage". e.g. when the Save participant fails today, it is usually possible to manually invoke Format Document or Quick Fix to get the save participant to execute. What about this "paste massager", how should this be exposed directly?

@DanTup
Copy link
Contributor Author

@DanTup DanTup commented Feb 3, 2021

This came up lately in LSP as well and from my experience implementing this for languages in the past I think there are more things to consider

capturing the imports on copy is not enough. Besides the fact that you might copy from another source the imports might not make any sense in the new paste location.

This sounds fairly server-implementation-specific and not something the editor should be concerned with. My intention wasn't just to copy the imports, but to delegate to the language server (which has a specific feature for producing metadata during a copy, and then producing edits given back that metadata during a paste).

When pasting, we can then read this metadata again. As part of this metadata, we could potentially store the URI and the range where the copy or cut event occurred in case this might be useful when pasting.

Would this be in addition to a copy event? To me, having access to this data at paste time seems a bit useless - the user could have copied the text and then deleted the file. Metadata about the imports needs to be gathered at copy time (and is something the language server needs to be involved in).

Also, making pasting async might become problematic w.r.t. timing and user expectation. What should happen when a new key press occurs after a paste and onWillPaste still hasn't returned.

If the worst case is that in this scenario the additional edits are completely lost and can't be re-applied, I don't think that's a deal breaker. You can always hit undo and then re-paste, without typing in between. I think users would much rather have this feature with limitations like that than not be able to have it at all.

@alexdima
Copy link
Member

@alexdima alexdima commented Feb 3, 2021

@DanTup In the first two paragraphs I attempt to reason why copy itself cannot be made async. Maybe we could add an onDidCopy that would execute immediately after the copy event is dispatched by the browser and handled by the editor. The language server would have a chance to compute and return some piece of data. We would store that piece of data until the next paste comes in, and if we know it was a paste from that specific copy event, we can match things up again. But unless we get some help from browsers, the data itself cannot be stored in the OS clipboard, only some identifier which can be used to correlate things.

If the worst case is that in this scenario the additional edits are completely lost and can't be re-applied, I don't think that's a deal breaker. You can always hit undo and then re-paste, without typing in between. I think users would much rather have this feature with limitations like that than not be able to have it at all.

I think a better strategy would be to give up on waiting for text edits from the language server and simply paste the text without the imports.

@DanTup
Copy link
Contributor Author

@DanTup DanTup commented Feb 3, 2021

In the first two paragraphs I attempt to reason why copy itself cannot be made async. Maybe we could add an onDidCopy that would execute immediately after the copy event is dispatched by the browser and handled by the editor. The language server would have a chance to compute and return some piece of data.

Yep, understood. I just wanted to highlight the importance of having a copy event of some sort (and that trying to make up for it only with paste would not work reliably). I think if there are caveats (again, like if the user modifies the document quickly, the results are invalid/discarded) that's entirely acceptable. Being able to bring imports >95% of the time is a significant important over 0%.

But unless we get some help from browsers, the data itself cannot be stored in the OS clipboard, only some identifier which can be used to correlate things.

Oh, I see - I thought the metadata was being suggested for use by the extension, but if you mean so VS Code can use it to locate which "extension data" corresponds to that (because it wasn't available at the time the clipboard was populated), that makes sense. I was reading entirely as an extension author and not considering the internal implementation :-)

I think a better strategy would be to give up on waiting for text edits from the language server and simply paste the text without the imports.

Yep, that's kinda what I meant - although I'd assumed the original text would be immediately pasted, and then the servers edits to insert imports (and potentially prefixes for type names in the pasted code, if required to avoid conflicts) applied afterwards. I think I'd personally prefer to see the text appear immediately and then an update fixing up the imports/prefixes than paste feel slow (a delay in pasting might lead to people hitting Ctrl+C again).

@333fred
Copy link
Member

@333fred 333fred commented Feb 3, 2021

I see a number of comments in this issue specifically focusing on imports as a reason for extra text edits to be applied after a paste event, and I just wanted to make sure that we're not considering that as the only reason why a user might want to have an on-paste handler. It might actually format the code in question, which causes other lines in the code to need to be reformatted. Or you could have a handler that, if you paste content into a string, does some form of character escaping for nested quote marks as another example. I admit I have trouble reasoning about an API that takes a paste edit and returns a possible series of edits. Does this need to include the original edit, or does it need to include an edit that matches the exact span of the original edit, with some other additional edits that can touch adjacent locations? Where does the cursor end up after? IMO the API would be significantly easier to implement (as a language server providing the handler) as a "here's the document that was just pasted into and here's the range of the text that was pasted in", and that's the approach I took with my LSP spec proposal: microsoft/vscode-languageserver-node#736.

Maybe we could add an onDidCopy that would execute immediately after the copy event is dispatched by the browser and handled by the editor. The language server would have a chance to compute and return some piece of data. We would store that piece of data until the next paste comes in, and if we know it was a paste from that specific copy event, we can match things up again.

I don't think this is a workable solution. Languages have many different ways of formatting import statements, even within a single language. You could copy/paste from one file without an alias to another file with an alias, and a smart formatter would want to be able to use those aliases in the pasted code. It's too dependent on the pasted-into context to be resolvable up front.

@mjbvz
Copy link
Contributor

@mjbvz mjbvz commented Feb 3, 2021

@dbaeumer The current api proposal flows looks something like:

  1. User copies text

  2. The extension is notified that a copy has happened. The extension generates an opaque blob of data that contains all the information to generate the imports on paste.

    With TypeScript for example, this blob would include the symbols and the files the came from. The important part is that the extension does not have to generate text to be pasted during this phase.

  3. The user pastes. Here the extension gets the opaque blob back and generate a workspace edit describe the text edit.

The extension would be responsible for handling cases such as: copy/paste between different projects, file changes between copy/paste, and so on.

I can talk with the TS team about onWillPaste vs onDidPaste to see if one would be easier to implement


@alexdima To workaround the async issue, I currently:

  1. When a copy happens, store a reference id in the clipboard. Let the copy event finish synchronously.

  2. Asynchronously go out to the extension to resolve the actual data to store. Map this to the reference id once we have it

  3. When a paste happens, look up the data to use based on the reference id in the clipboard. Using this, let the extension handle the rest of the paste

@333fred
Copy link
Member

@333fred 333fred commented Feb 4, 2021

I can talk with the TS team about onWillPaste vs onDidPaste to see if one would be easier to implement

For reference of an existing implementation, I want this to be able to expose the same Roslyn functionality that already exists in VS (http://sourceroslyn.io/#Microsoft.CodeAnalysis.EditorFeatures/Implementation/Formatting/IEditorFormattingService.cs,35), which is structured as a onDidPaste.

@dbaeumer
Copy link
Member

@dbaeumer dbaeumer commented Feb 4, 2021

@mjbvz Regarding

With TypeScript for example, this blob would include the symbols and the files the came from. The important part is that the extension does not have to generate text to be pasted during this phase.

I don't think that this is the expensive part. The expensive part IMO is to find out whether these imports still make sense at the pasted location. IMO we can't insert them blindly.

@mjbvz mjbvz removed this from the February 2021 milestone Feb 17, 2021
@mjbvz mjbvz added this to the March 2021 milestone Feb 17, 2021
@mjbvz mjbvz removed this from the March 2021 milestone Mar 19, 2021
@mjbvz mjbvz added this to the April 2021 milestone Mar 19, 2021
@mjbvz mjbvz removed this from the April 2021 milestone Apr 20, 2021
@mjbvz mjbvz added this to the May 2021 milestone Apr 20, 2021
@mjbvz mjbvz removed this from the May 2021 milestone May 19, 2021
@mjbvz mjbvz added this to the On Deck milestone May 19, 2021
carlocardella pushed a commit to carlocardella/vscode-TextToolbox that referenced this issue May 31, 2021
@pouyarezvani

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api editor-code-actions feature-request
Projects
None yet
Development

No branches or pull requests

8 participants