gptel-transient: Fix display issues with evil visual selections#1236
Open
real-or-random wants to merge 2 commits intokarthink:masterfrom
Open
gptel-transient: Fix display issues with evil visual selections#1236real-or-random wants to merge 2 commits intokarthink:masterfrom
real-or-random wants to merge 2 commits intokarthink:masterfrom
Conversation
Owner
|
This looks great, thanks for the PR! I'll merge it soon. |
* gptel.el: Bump minimum transient version from 0.7.4 (2024-08-05) to 0.7.8 (2024-11-02). This will be required for :environment as used in then next commit. * README.org: Same. * NEWS: Mention this change.
* gptel-transient.el (gptel--transient-fix-evil-visual): Add this transient environment function whose purpose is to ensure that the region matches the visual selection in evil-mode before calling transient display code (karthink#909). This relies on a feature present in transient>=0.7.8, see: magit/transient@05c011b * gptel-transient.el (gptel-menu): Use gptel--transient-fix-evil-visual as :environment. * gptel-rewrite.el (gptel-rewrite): Use gptel--transient-fix-evil-visual as :environment.
real-or-random
force-pushed
the
202602-evil-visual-env
branch
from
8ae1d51 to
1581bc0
Compare
Contributor
Author
|
Force-pushed to fix a typo in the docstring and to rebase on master |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #909 and a few other issues, e.g., missing "Rewrite r" choices in the transient menu when there's an evil visual selection.
I played around with multiple approaches, but I think this is by far the cleanest fix. First, it kills the problem entirely, i.e., we won't need to change every
(use-region-p),(region-beginning), etc. responsible for displaying a transient, which would be very annoying to maintain. Second, this approach is not very hacky because it uses a mechanism offered by transient for setting up an "environment" in which the display code runs.We'll need transient 0.7.8. I think it's fine to bump it -- the release dates differ just by a few months -- but we can also make the code conditional in the version if you prefer that approach.
I couldn't notice any difference in performance, and performance should anyway be totally fine:
evil-visual-expand-regiondoes is to check if there's a visual selection, i.e., if there's anything to do at all. If not, it returns immediately. (Note that these evil functions are performance-critical in evil-mode itself because they run in the pre/post-command-hook.)