Skip to content

fix: properly handle buffer deletion #68

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

Merged
merged 1 commit into from
Jun 3, 2025
Merged

Conversation

tomtomjhj
Copy link
Contributor

Problems

  • If dap-view buffer is :bwipeout-ed, subsequent close() raises invalid buffer error while attempting to nvim_buf_delete (= bwipeout) the buffer because the buffer is not removed from the state.
  • quit_buf_autocmd is weird. BufDelete is invoked when a buffer becomes unlisted. So, since dap-view buffer is already unlisted, closing it with commands like :q does not invoke the BufDelete autocmd.

Solution:

  • Let close() check the validity of the buffer first.
  • Use BufWipeout autocmd.
  • Let the BufWipeout callback first remove the buffer from dap-view state to avoid deleting the buffer inside BufWipeout (E937).

@igorlfs
Copy link
Owner

igorlfs commented Jun 3, 2025

Not gonna lie, I'm not an expert when it comes to wiping out buffers and such.

quit_buf_autocmd is weird.

Indeed, I yoinked it from nvim-tree, and then made some modifications. Do you think it's possible to get rid of it completely? We could create autocmd with a pattern to match dap-view's buffers.

Problems
* If dap-view buffer is :bwipeout-ed, subsequent close() raises invalid
  buffer error while attempting to nvim_buf_delete (= bwipeout) the
  buffer because the buffer is not removed from the state.
* quit_buf_autocmd is weird.
  BufDelete is invoked when a buffer becomes unlisted.
  So, since dap-view buffer is already unlisted, closing it with
  commands like :q does not invoke the BufDelete autocmd.

Solution:
* Let close() check the validity of the buffer first.
* Use BufWipeout autocmd.
* Let the BufWipeout callback first remove the buffer from dap-view
  state to avoid deleting the buffer inside BufWipeout (E937).
@tomtomjhj
Copy link
Contributor Author

Not gonna lie, I'm not an expert when it comes to wiping out buffers and such.

haha terms used in nvim's api is somewhat different from the legacy vim stuff, adding more confusion to the already confusing vim autocmd system.

Do you think it's possible to get rid of it completely? We could create autocmd with a pattern to match dap-view's buffers.

That would also work, as long as it is a BufWipeout. What I thought is weird was that it was using BufDelete, which won't be triggered at all. I believe the fixed implementation I proposed in this PR should also be fine.

@igorlfs
Copy link
Owner

igorlfs commented Jun 3, 2025

Thanks for the contribution!

For now, we'll stick with the quit_buf_autocmd. I may refactor that when we finally get multi session support.

@igorlfs igorlfs merged commit 643345a into igorlfs:main Jun 3, 2025
1 check passed
@tomtomjhj tomtomjhj deleted the bwipeout branch June 4, 2025 00:19
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