-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Allow pasting registers from QuickPick menu #9730
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
base: master
Are you sure you want to change the base?
Conversation
src/cmd_line/commands/register.ts
Outdated
| } | ||
|
|
||
| private static async paste(vimState: VimState, item: RegisterDisplayItem) { | ||
| // TODO: Can I reuse PutCommand here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paste method feels wrong. I don't want to have to reinvent the wheel just to actually paste register content. I would like to reuse existing functionality, but I couldn't figure out the correct way to do that.
What I tried was to create an instance of PutCommand here and pass it the register key as part of its arguments, but somehow that didn't give me the desired result (I had to press ESC after clicking the paste button, for the text to show up).
Perhaps someone can give me some pointers for how to implement this properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to press ESC after clicking the paste button, for the text to show up
Executing the command from a callback makes things a bit weird, since you're outside the normal loop of actions modifying VimState then rendering the new changes to the VSCode editor.
The cleanest way to go IMO is to make a promise that resolves in the callbacks you give to onDidChangeSelection and onDidTriggerItemButton and await it. Also make the existing void promise;s into awaits. Basically - pause the Vim state machine until the user does something with the quickpick. Then you can use PutCommand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed that seems to do the trick quite nicely! Thanks. I pushed another commit to implement that approach.
Note that in my latest commit I removed the "Paste" button from the information message which is shown when a user clicks one of the quick pick items. I did that because I couldn't find a nice way to resolve the promise when the user closes the message without clicking the button. But I think usability-wise that doesn't make a big difference.
What this PR does / why we need it:
This PR adds extends the
:displaycommand, so that the QuickPick which opens to show the registers also offers action buttons to paste the register content. This is equivalent to running:di, followed by:putwith the selected register. While the functionality is not new per se, it makes the QuickPick of the registers slightly more useful.In order to do this, most of the changes are within
registers.tsbut some small changes were made to theRegisterclass as well. These are convenience functions, and they do not alter any existing functionality.Which issue(s) this PR fixes
I couldn't find an issue which talks about this feature. If a linked issue is required, I can open one afterwards and discussion can happen there, but at least for me this feature would improve my productivity greatly.
Special notes for your reviewer:
I know it's a mouse action rather than a keyboard action. The Vim gods shall have mercy on my soul.