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

Make gptel-rewrite text replacement preserve evaporative overlays #512

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daedsidog
Copy link
Contributor

@daedsidog daedsidog commented Dec 20, 2024

I am working on gptel integration into my extension.

gptel-rewrite is very nice and already does a lot of work such as adding the ability to diff/ediff changes. Though most of the functions are internal to gptel, I am planning, for now, to use gptel--rewrite-dispatch to create the overlays, until a better solution.

I've played around with it and it integrates very nicely, except for one issue: since I am using pre-existing overlays (not in any way related to the overlays gptel-rewrite creates), they are deleted because they have evaporate set to true when gptel-rewrite replaces the accepted text.

To circumvent this, I made the text replacement unobtrusive, by first inserting the new text, then deleting the old, instead of vice versa.

Besides the changes of convenience for my extension, it seems, to me, to also be desirable behavior, but I'll leave you to judge. My only concern is that it goes against some sort of standard convention that replacement always means total deletion and insertion.

@daedsidog daedsidog changed the title gptel-rewrite: Make text replacement preserve overlays Make gptel-rewrite text replacement preserve evaporative overlays Dec 20, 2024
@daedsidog daedsidog force-pushed the feature-unobtrusive-rewrite-accept branch from 14f77bf to 1d91115 Compare December 20, 2024 18:35
@karthink
Copy link
Owner

I've played around with it and it integrates very nicely, except for one issue:
since I am using pre-existing overlays (not in any way related to the overlays
gptel-rewrite creates), they are deleted because they have evaporate set to
true when gptel-rewrite replaces the accepted text.

This is by design -- the rewrite "transaction" ends when you accept or reject
the text. Otherwise there would be no way to actually get rid of the overlay.

Besides the changes of convenience for my extension, it seems, to me, to also be
desirable behavior, but I'll leave you to judge. My only concern is that it goes
against some sort of standard convention that replacement always means total
deletion and insertion.

Could you explain why keeping the overlay around after accepting the response is
desirable behavior? I've actually been considering deleting the overlay after
running any operation, including diff/ediff.

@daedsidog
Copy link
Contributor Author

daedsidog commented Dec 22, 2024

@karthink There is a misunderstanding, I am not talking about the gptel-rewrite overlay, which should rightly be removed.

I am talking about existing, unrelated overlays in the buffer, surrounding text. I want to replace the text of those overlays, and those overlays are evaporative.

Here's the behavior I wanted:

overlays

Previously, this would delete the existing overlay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants