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

Force buffer delete for terminal #1943

Merged

Conversation

TheMeaningfulEngineer
Copy link
Contributor

@TheMeaningfulEngineer TheMeaningfulEngineer commented May 18, 2022

Closes #1940

Acceptance criteria listed in the issue.

@TheMeaningfulEngineer
Copy link
Contributor Author

@Conni2461
I was hoping to add some prettier error handling for cases where one tries to close an unsaved buffer.
If you try doing it now there is a lot of ugly red errors printed.

Do you know how can the errors from vim.api.nvim_buf_delete(selection.bufnr, { force = force }) be handled to show a human friendlier error?

I've tried wrapping it with pcall but that didn't prevent the errors from printing.

@Conni2461
Copy link
Member

Doing

pcall(vim.api.nvim_buf_delete, selection.bufnr, { force = force }) goes from this error:

E89: No write since last change for buffer 18 (add ! to override)
E5108: Error executing lua [string ":lua"]:1: Failed to unload buffer.
stack traceback:
        [C]: in function 'nvim_buf_delete'
        [string ":lua"]:1: in main chunk

to this

E89: No write since last change for buffer 18 (add ! to override)

I think the latter one in an improvement, and you could add the pcall. You can't catch that error because pcall only catches lua errors and this seems to come from somewhere in neovim (old viml land).

@TheMeaningfulEngineer
Copy link
Contributor Author

TheMeaningfulEngineer commented May 21, 2022

Thanks @Conni2461.
However enclosing the call with pcall doesn't change the verbosity level for me.
I've pushed the second commit with this.
This code produces the exact same error as when pcall isn't present.

Does running this on your machine gives you the reduced error log?

My bad, totally missed the way pcall handles parameters :)

@TheMeaningfulEngineer
Copy link
Contributor Author

@Conni2461
Hey, can this be merged?

TheMeaningfulEngineer and others added 3 commits May 29, 2022 19:49
The motivation behind this logic came from the use case of deleting
buffers.
Before this commit this was the behaviour:
* try to delete a buffer which wasn't saved
* error pops up, buffer not deleted
* regardless of the error the selection gets removed from telescope's
  list

After this commit:
* try to delete a buffer which wasn't saved
* error pops up, buffer not deleted
* regardless of the error the selection gets removed from telescope's
@Conni2461 Conni2461 force-pushed the delete-buffer-terminal branch from ee61119 to 055e006 Compare May 29, 2022 17:49
@Conni2461 Conni2461 changed the base branch from master to dev May 29, 2022 17:49
@Conni2461
Copy link
Member

Thanks for the PR :) i cleaned it up, wrote docs and changed the target branch to the dev branch following #1938 because i dont consider this a fix and only fixes are currently merged into master (you've also opened this as feature request). It will hit master in about 2 weeks.

Thanks again for improving actions.delete_buffer as well as Picker:delete_selection

@Conni2461 Conni2461 merged commit 14ab37a into nvim-telescope:dev May 29, 2022
Conni2461 pushed a commit that referenced this pull request Jun 2, 2022
Conni2461 pushed a commit that referenced this pull request Jun 6, 2022
Conni2461 pushed a commit that referenced this pull request Jun 12, 2022
Conni2461 pushed a commit that referenced this pull request Jun 15, 2022
Conni2461 pushed a commit that referenced this pull request Jun 18, 2022
Conni2461 pushed a commit that referenced this pull request Jun 30, 2022
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.

Make delete_buffer force deletion for terminal windows
2 participants