-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
[PastePlain] Introduce Paste as Plain Text module #23645
Conversation
This comment has been minimized.
This comment has been minimized.
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.
Questions/Comments
- Need to replace the
PastePlainLogo
with an actual new icon- requested resources from Ethan in an email thread. I'll update them on the PR once I get them.
- Need to figure out telemetry
- just need to figure out how to track enablement/disablement.
Probably easier once I update the settings UIYup, it was in there.
- just need to figure out how to track enablement/disablement.
- Need to fix Ctrl+Win+V not working
so, when I first implemented this, I thought the keybinding was supposed to be Win+Shift+V. I went back and tried to replace Win+Shift+V with Ctrl+Win+V at the end, but it seems to not be working. Idk where that's set up. Any ideas?- EDIT: fixed! the win+shift+v was written to the settings.json. As expected, if you delete the settings.json, it'll revert it back to the default value of ctrl+win+v (as desired).
- if you see any
Text Extractor
references, let me know. Probably missed them. - It seems like the first time I do the keybinding, it doesn't work (or it has a decent delay). Not sure if this is due to debug mode. I'll keep an eye out for that as we move forward. Totally possible I did something wrong.
- EDIT: seems to be due to debug mode. Resolving.
// TODO CARLOS: I just used a new GUID here. I don't know if this is the right way to do it. | ||
const wchar_t SHOW_PASTEPLAIN_SHARED_EVENT[] = L"Local\\PastePlainEvent-57dc5a03-20b4-423b-aa5c-574e809d95a7"; |
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.
Want to bring attention to this. Let me know if the guid should be generated/inserted in a specific way. I just generated a new one using VS.
inline gpo_rule_configured_t getConfiguredPastePlainEnabledValue() | ||
{ | ||
return getConfiguredValue(POLICY_CONFIGURE_ENABLED_PASTE_PLAIN); | ||
} |
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.
Any way I can test this and make sure I did it right?
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.
@carlos-zamora
I think the only way to test it is by setting the registry key manually.
@@ -0,0 +1,1211 @@ | |||
// Copyright (c) Microsoft Corporation |
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 is a big file. Had to introduce a lot of stuff to get SendInput()
to work (particularly to define what an INPUT
is).
_activationShortcutPressed = true; | ||
e.Handled = true; | ||
{ | ||
var data = Clipboard.GetContent(); |
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 here is really the interesting part of the PR. 😉
{ | ||
var package = new DataPackage(); | ||
package.SetText(text); | ||
if (Clipboard.SetContentWithOptions(package, new ClipboardContentOptions() { IsAllowedInHistory = false, IsRoamable = false })) |
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.
Idk if we want to make IsRoamable
configurable? 🤷
Love this! Very useful!
|
So 🔥 |
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.
Getting ready to test it out, but here are changes I had to make in order for compilation :)
src/modules/pasteplain/PastePlainModuleInterface/PastePlainModuleInterface.vcxproj
Outdated
Show resolved
Hide resolved
src/modules/pasteplain/PastePlainModuleInterface/PastePlainModuleInterface.vcxproj
Outdated
Show resolved
Hide resolved
src/modules/pasteplain/PastePlainModuleInterface/PastePlainModuleInterface.vcxproj
Outdated
Show resolved
Hide resolved
src/modules/pasteplain/PastePlainModuleInterface/PastePlainModuleInterface.vcxproj
Outdated
Show resolved
Hide resolved
src/modules/pasteplain/PastePlainModuleInterface/PastePlainModuleInterface.vcxproj.filters
Outdated
Show resolved
Hide resolved
src/modules/pasteplain/PastePlainModuleInterface/PastePlainModuleInterface.vcxproj.filters
Outdated
Show resolved
Hide resolved
Tested and verified that it pastes rich text as plain text using Ctrl+Win+V. \o/ |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Updated the settings UI and OOBE, but when I try to deploy the settings UI project I get a popup saying "The application cannot be run as a standalone process. Please start the application through the runner.". How can I validate the settings UI and OOBE? |
Option 1 Option 2 |
Weird. "Paste as Plain Text" won't pop up in the settings UI when I deploy PowerToys.Settings. If I try to build the complete solution, the "Hosts" project fails: Build log for failure in Hosts project
But I get the same issue when building the solution off of Any ideas how to fix this? |
@carlos-zamora thats awkward 😓 Are Hosts tests failing or not building? I am confused since Hosts doesn't build. |
@carlos-zamora Update that .NET 7 SDK to 7.0.102 ;). You're still running the .NET 7 Preview 7 SDK. |
Yup, that fixed the build for me. Thanks! Still not seeing Paste As Plain Text pop up in the settings UI or the OOBE though. I was hoping this would fix it. Any ideas? |
Hi @carlos-zamora , |
Added another commit to add Paste as Plain text to the new quick access flyout as well :) |
Regarding the feature itself, doesn't seem to be working for me when I start PowerToys. Is it supposed to work out of the box? |
Thank you!! Yeah this is weird. It works when I'm debugging the feature itself, but it looks like when I have the settings app open, it won't actually enable/disable it. Probably something I messed up in the settings app. Investigating. |
The assertion thing is a known bug that happens only when PT was build in debug mode: #15139 => #15139 (comment) |
@jaimecbernardo can you help here? |
is this getting near ready to merge in? :) |
Yes, Almost done with the wrap up. Will need a review after that, though ;) |
All wrapped up and ready to review! |
This comment has been minimized.
This comment has been minimized.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Looks good, works good! Nice job!
One note: when I want to paste something a few times in a row using standard Ctrl+V, I'd hold Ctrl and pres V a few times. I'd expect this to work the same, but it doesn't. I have to press the entire hotkey all over. Is that intended?
Thanks for the review. |
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.
* [PastePlain] Introduce Paste as Plain Text module * fix build * add telemetry * update settings UI * spell * Add navigation items to Settings and OOBE * Add PastePlain to the Quick Access flyout * try to fix PastePlain not being enabled from runner/settings * load dll properly * installer files * Add PastePlain project name * Use win32 APIs in the module interface instead * Fix spellcheck * Fix build errors * Add success, error and invoke telemetry * Add Settings Telemetry * Add GPO definitions * Fix analyzer errors * Use static_cast instead of reinterpret_cast * Add images to Settings * Add note about replacing clipboard contents * Fix learn more text * Add link to readme * Remove unneeded C# app * Fix installer * Fix spellchecker --------- Co-authored-by: Jaime Bernardo <jaime@janeasystems.com>
Summary of the Pull Request
Introduces the "Paste as Plain Text" module which allows the user to paste their most recent clipboard content as plain text (as opposed to formatted text).
PR Checklist
Detailed Description of the Pull Request / Additional comments
This is heavily based on the work done for the "Text Extractor" module then ripping out anything that may be unnecessary. The same KeyboardMonitor is implemented and used. The interesting logic really falls inside the
KeyboardMonitor
as that is where the clipboard is opened/read/modified, then a ctrl+v input sequence is injected.The default key binding for this will be ctrl+win+v.
Validation Steps Performed