-
Notifications
You must be signed in to change notification settings - Fork 302
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
[vi-mode] Add ViClipboardMode Option to Support Copying and Pasting to System Clipboard #3769
base: master
Are you sure you want to change the base?
Conversation
@microsoft-github-policy-service agree |
This option allows the user to choose if they want the ViRegister to use the system clipboard or the default local clipboard. By setting ViClipboardMode to SystemClipboard, the user can easily copy and paste text in the PowerShell prompt using Vi shortcuts in Command mode.
e4bd694
to
585d2c2
Compare
When I initially introduced the In my mind, this would have been done using the May I respectfully ask what you think ? Would you mind me proposing my help to you on this feature ? I see that you have done a ton of work for the documentation already, so this maybe a bit too late on my part… 😥 |
The docs weren't a lot of work, so I don't mind changing the feature and redoing them. I've been using my branched PSReadLine personally since I posted the pull request. It works well, but there are some situations that have been annoying. If I delete a character, word, or line, the text always gets copied to the clipboard with my changes. Occasionally I actually do want to copy the text that I delete, but most of the time I don't. All of that to say... I want to change how my feature works before merging. What if we did this compromise solution?
Reasons why I like this solution:
Thoughts? |
@WestRyanK, thank you for your feedback. That is an interesting proposal and I understand what you are trying to achieve. I do have some concerns though. Mainly, I think if we strive to emulate the VIM keybindings and behaviour, using named registers would be the least surprising option. Deviating from this would create a kind of vim-but-not-quite-like emulation that may create a precedent for other behaviours that future users may want to include in all sorts of crazy ways. Second, I think what you want could be achieved by using the builtin key-rebinding capabilities of PSRL. That is, you could create a custom profile that reassigns whatever key bindings you would like to the appropriate function or sequence of functions to use the default or the named register depending on your needs. For instance, you would like to reassign yy to use the system clipboard instead of its default internal register. If we are careful enough to support public functions, that could be achieved using something like the following script in your Set-PSReadLineKeyHandler `
-ViMode Command `
-Chord 'y,y' `
-BriefDescription "Copy to clipboard" `
-LongDescription "Yank a set of lines to the system clipboard" `
-ScriptBlock {
param($key, $arg)
[Microsoft.PowerShell.PSConsoleReadLine]::ViSelectRegister($key, "+")
[Microsoft.PowerShell.PSConsoleReadLine]::ViYankLine($key, $arg)
} We could also include those snippets into the The way I see for implementing this would be:
private readonly ViRegister _clipboard;
private readonly ViRegister _systemClipboard;
private readonly IDictionary<string, ViRegister> _registers;
// ctor
static PSConsoleReadLine()
{
_clipboard = new ViUnnamedRegister(_singleton);
_systemClipboard = new ViSystemClipboard(_singleton);
_registers = new Dictionary<string, ViRegister>{
[""] = _clipboard,
["\""] = _systemClipboard,
["+"] = _systemClipboard,
}
}
At this point, I think we would be mostly done. It would be very easy to actually add named registers in the future: i.e creating public functions to create new registers on the fly but I would refrain from this in this PR. Please, let me know how you feel about this. |
Oh, I started implementing the feature based on your comments before I realized that you had actually implemented it yourself. I suppose I'll let you finish it then? |
I would be more comfortable leaving the spirit of the changes to you. |
I'm also wanting this ViClipboardMode as must have feature. What's needed to be done, to merge the PR? If any help is needed, I'm eager to contribute. |
I think @WestRyanK would rather have me merge my contributions as a separate PR. I’m not saying my suggestions are the only good way to go for implementing this feature. If @luqsthunder you have the time and energy to do this more quickly than I can, please, feel free to work on this and submit a PR yourselves. Then I will help you lobby the maintainer team members here for merging. 😁 I think I cannot reasonably dedicate some time for this before at least a couple of weeks. |
@luqsthunder I have submitted #4220 for review. |
PR Summary
This pull request adds the ViClipboardMode option. This option has two values: ViRegister and SystemClipboard.
ViRegister, the default, maintains the original behavior for copying and pasting in Vi edit mode, which was to copy and paste using a buffer that was local to the current instance of PSReadLine. With this original behavior, text cannot be copied to/from other applications using Vi keys and it can't be copied between separate panes or windows of PowerShell.
The other value, SystemClipboard, changes the copy and paste behavior in Vi edit mode. When this value is used, text is copied and pasted from the system's clipboard when the user uses Vi key commands. This allows the user to copy and paste text from external applications using "y" or "p". It also allows the user to copy and paste text between different panes or windows of PowerShell.
PR Checklist
Microsoft Reviewers: codeflow:open?pullrequest=#3769