[Cmd] use prompt-toolkit native clipboard integration#1721
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1721 +/- ##
=======================================
Coverage 99.61% 99.61%
=======================================
Files 23 22 -1
Lines 5915 5930 +15
=======================================
+ Hits 5892 5907 +15
Misses 23 23
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
|
@neoniobium You'll need to merge Btw, you should now be able to create branches in the main repo, so you don't need to use a Fork anymore unless you want to. Using branches in main repo, makes it easier for others to help collaborate. |
Ok, next time I will create a branch instead of using my frok. |
|
Thanks again for the review. I was also thinking whether the clipboard should be shared will each session, or example Furthermore I was wondering |
|
@kmvanbrunt Do you have any thoughts on this PR? |
|
|
||
| # Only enable PyperclipClipboard if the system clipboard is accessible to Pyperclip. | ||
| try: | ||
| cb = PyperclipClipboard() |
There was a problem hiding this comment.
PyperclipClipboard is instantiated even if allow_clipboard is False.
The docstring states that setting allow_clipboard to False disables clipboard interactions. However, PyperclipClipboard is currently added to the PromptSession regardless of this flag (if the system clipboard is accessible). This natively enables system clipboard pasting inside the prompt, which contradicts the intention of disabling all clipboard interactions. If allow_clipboard is meant to disable all system clipboard access, you should conditionally check self.allow_clipboard. (Note: If you apply this fix, you will also need to update test_init_with_no_clipboard_allowed).
Recommend wrapping code ath initializes cb inside a check:
if self.allow_clipboard:
try:
cb = PyperclipClipboard()
...| """Custom exception class for when redirecting or piping output fails.""" | ||
|
|
||
|
|
||
| class ClipboardError(Exception): |
There was a problem hiding this comment.
ClipboardError will cause a generic stack trace since it is not caught in onecmd_plus_hooks.
ClipboardError inherits directly from Exception. Because there is no explicit except ClipboardError: block in onecmd_plus_hooks (where commands are executed), raising this error during output redirection will trigger the generic except Exception: handler, which prints a full stack trace via self.pexcept(ex).
You should either catch ClipboardError gracefully in onecmd_plus_hooks alongside RedirectionError, or simply have this class inherit from RedirectionError so it gets handled cleanly by the existing logic, i.e.:
class ClipboardError(RedirectionError):
"""Custom exception class for when clipboard operations fail."""|
@neoniobium I pushed an update to your branch that merged in changes from |
Addresses #1720