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

Actually fully disable system clipboard (#5155) #5261

Merged
merged 2 commits into from Jan 15, 2024

Conversation

azertyfun
Copy link
Contributor

Fixes #5155

In VI mode, disabling "delete" hotkeys isn't enough.

However not using the PyperclipClipboard seems like a better solution to the problem anyway (at least to me, maybe I missed something as I am far from a PTK specialist!).

The configuration variable has been renamed for consistency, though the old one still works.

For community

⬇️ Please click the 👍 reaction instead of leaving a +1 or 👍 comment

@azertyfun azertyfun force-pushed the disable-system-clipboard branch 2 times, most recently from c381fb6 to 3da754a Compare January 8, 2024 10:59
@anki-code
Copy link
Member

anki-code commented Jan 8, 2024

hi! Thanks you for working on this!
I'm not the user of VI mode and maybe my question irrelevant but how this affect on readline shell (xonsh --shell-type readline)?

@anki-code
Copy link
Member

Ah, I see this works only in PTK. No questions.

@anki-code anki-code added the vi label Jan 8, 2024
xonsh/environ.py Show resolved Hide resolved
xonsh/ptk_shell/shell.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jan 15, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (66f0ba3) 67.16% compared to head (277861d) 65.12%.
Report is 1 commits behind head on main.

Files Patch % Lines
xonsh/ptk_shell/shell.py 60.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5261      +/-   ##
==========================================
- Coverage   67.16%   65.12%   -2.05%     
==========================================
  Files         119      119              
  Lines       23124    23099      -25     
  Branches     4855     4851       -4     
==========================================
- Hits        15532    15043     -489     
- Misses       6390     6842     +452     
- Partials     1202     1214      +12     
Flag Coverage Δ
macOS-latest 64.43% <66.66%> (-0.03%) ⬇️
ubuntu-latest 64.74% <66.66%> (-0.02%) ⬇️
windows-latest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jnoortheen
Copy link
Member

I'd suggest adding a separate config variable to control system clipboard integration if there is not one already. This on_delete function controls behaviour of all kill-ring operations

azertyfun pushed a commit to azertyfun/xonsh that referenced this pull request Jan 15, 2024
Fixes xonsh#5155

In VI mode, disabling "delete" hotkeys isn't enough.

After discussion in xonsh#5261, we don't change the default behavior but
allow fully disabling the system clipboard to better support the
use-case of VI mode user.
@azertyfun
Copy link
Contributor Author

Alright, thanks for the feedback. I've rebased the PR to keep using the old code.

There is now no behavior change for users unless they actively set $XONSH_USE_SYSTEM_CLIPBOARD = False.

Fixes xonsh#5155

In VI mode, disabling "delete" hotkeys isn't enough.

After discussion in xonsh#5261, we don't change the default behavior but
allow fully disabling the system clipboard to better support the
use-case of VI mode user.
@jnoortheen jnoortheen merged commit 5b20891 into xonsh:main Jan 15, 2024
15 checks passed
@jnoortheen
Copy link
Member

Thanks @azertyfun !

theadam pushed a commit to theadam/xonsh that referenced this pull request Mar 11, 2024
Allow to fully disable system clipboard

Fixes xonsh#5155

In VI mode, disabling "delete" hotkeys isn't enough.

After discussion in xonsh#5261, we don't change the default behavior but
allow fully disabling the system clipboard to better support the
use-case of VI mode user.

Co-authored-by: Nathan Monfils <nmo@escaux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$XONSH_COPY_ON_DELETE = False not respected when $VI_MODE = True
4 participants