helix-term/commands: implement buffer-close (bc, bclose) #1035
Conversation
|
Just realized I should probably also implement |
|
(Invited you to the Matrix Space too) |
cole-h
force-pushed
the
buffer-close
branch
2 times, most recently
from
1e9c14b to
e51386c
Compare
According to archseer, this was never implemented or used properly. Now that we have a proper "buffer close" function, we can get rid of this.
This behaves the same as Kakoune's `delete-buffer` / `db` command:
* With 3 files opened by the user with `:o ab`, `:o cd`, and `:o ef`:
* `buffer-close` once closes `ef` and switches to `cd`
* `buffer-close` again closes `cd` and switches to `ab`
* `buffer-close` again closes `ab` and switches to a scratch buffer
* With 3 files opened from the command line with `hx -- ab cd ef`:
* `buffer-close` once closes `ab` and switches to `cd`
* `buffer-close` again closes `cd` and switches to `ef`
* `buffer-close` again closes `ef` and switches to a scratch buffer
* With 1 file opened (`ab`):
* `buffer-close` once closes `ab` and switches to a scratch buffer
* `buffer-close` again closes the scratch buffer and switches to a new
scratch buffer
Namely, if you have a document open in multiple splits, all the splits will be closed at the same time, leaving only splits without that document focused (or a scratch buffer if they were all focused on that buffer).
|
OK, I think I've got this in a good place (functionality-wise) now. That said, I feel like the code can be improved -- I just don't know how, yet. For the most part, I've taken inspiration from how |
helix-view/src/editor.rs
Outdated
| } | ||
|
|
||
| if view.doc == doc_id { | ||
| let new_id = if let Some(alt) = view.last_accessed_doc { |
There was a problem hiding this comment.
For the most part, I've taken inspiration from how nvim handles splits (at least, in my neovim with its various plugins.....) -- namely, that closing a buffer that's visible in multiple splits will also close those splits.
This actually doesn't seem to do that but instead tries to display a different document in the split? Might be a bit better actually since it keeps the layout intact.
If you'd like to keep the vim behaviour on the other hand, then this code can be simplified: simply self.close all the views where view.doc == doc_id, then remove the document from self.documents. If you're left with an empty view tree but !self.documents.is_empty(), then push a new view and set it to self.documents.iter().first()
There was a problem hiding this comment.
The last_accessed_doc was so that we could switch to that doc if it was Some(...), since it's more likely that the user wants to see the document they were looking at before closing their current one, than it is that they want to see the first document...
By unconditionally closing the view, we can't check the last_accessed_doc and switch to that if it exists... So opening ab, cd, and ef, then closing ef will bring you to ab instead of (as previously) cd. This seems weird to me.
|
Thanks! I was tremendously stuck with my attempt to implement this. |
|
Just a note that the commit body is now wrong -- we decided to go a simpler route, so closing a buffer will bring you to the first document in the list, not the last accessed document, or the document before it in the list. |
This behaves the same as Kakoune's
delete-buffer/dbcommand::o ab,:o cd, and:o ef:buffer-closeonce closesefand switches tocdbuffer-closeagain closescdand switches toabbuffer-closeagain closesaband switches to a scratch bufferhx -- ab cd ef:buffer-closeonce closesaband switches tocdbuffer-closeagain closescdand switches toefbuffer-closeagain closesefand switches to a scratch bufferab):buffer-closeonce closesaband switches to a scratch bufferbuffer-closeagain closes the scratch buffer and switches to a newscratch buffer
Closes #851.
I'm new to the codebase (as you can see by this being my first contribution...), so I may have done some things "wrong". Please correct me on everything, no matter how small.
Note that I publicized the
jumpsfield of theJumpListstruct inhelix-view/src/view.rsin order to ensure that no references to the now-removed buffer exist anywhere. I took a naive approach at validating this: I simply used the debug formatter to print thecompositor::Contextstruct and looked for any references to the now-removed document's ID.If you'd prefer, I could instead make a method that would give access (mutable and not) to this field.
Also note that I named the command
buffer-closeinstead of onlybclose(but I did add this andbcas an alias) orclosein order to pave the way for potentially more buffer-specific actions. Namespacing is easier at implementation time than at some point in the future where we wished we had done this.