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

Debouncing is negated because of infoview updates #115

Open
rish987 opened this issue Aug 14, 2021 · 3 comments
Open

Debouncing is negated because of infoview updates #115

rish987 opened this issue Aug 14, 2021 · 3 comments
Labels
bug Something isn't working infoview Relates to infoview

Comments

@rish987
Copy link
Collaborator

rish987 commented Aug 14, 2021

At the moment, debouncing text changes is kind of pointless, since we're updating the infoview (and hence making a request) on CursorMovedI, which will force textDocument/didChange.

Following #113, it should be safe to remove CursorMovedI entirely, because we'll be updating on textDocument/didChange, and moving the cursor entirely within insert mode without changing the text seems to be a misuse of vim anyways (from my experience, but let me know if I'm wrong about this).

@rish987 rish987 added enhancement New feature or request infoview Relates to infoview labels Aug 14, 2021
@rish987
Copy link
Collaborator Author

rish987 commented Aug 14, 2021

Correction: I think we should replace CursorMovedI with InsertEnter to allow for things like normal commands A/I and insert mode command <C-o>... to update the infoview. CursorMovedI just seems like something best avoided if possible, it could get called too often with too many other things happening. We would have a similar issue with CursorHoldI if the debounce timer is longer than the hold timer.

@rish987
Copy link
Collaborator Author

rish987 commented Aug 14, 2021

Actually, the above would likely involve adding something like a current field to the current pin so that we would know to update the position in addition to making an update, which I really wanted to avoid doing (i.e. not give special status to the current pin). So I think the best way here is to keep things as is, but make yet another upstream change that configurably allows client.request to abort postpone a request rather than flushing changes. Will try this out with #113 soon.

@rish987 rish987 changed the title Remove CursorMovedI autocmd when debouncing (and possibly entirely) Abort infoview updates when debouncing text changes Aug 14, 2021
@rish987 rish987 added bug Something isn't working and removed enhancement New feature or request labels Aug 14, 2021
@rish987
Copy link
Collaborator Author

rish987 commented Aug 14, 2021

(considering this a bug rather than enhancement since people should expect setting debounce_text_changes to work regardless of the timeout.)

@rish987 rish987 changed the title Abort infoview updates when debouncing text changes Postpone infoview updates when debouncing text changes Aug 14, 2021
@rish987 rish987 changed the title Postpone infoview updates when debouncing text changes Debouncing is negated because of infoview updates Aug 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working infoview Relates to infoview
Projects
None yet
Development

No branches or pull requests

1 participant