Add post_close_float()

[christoffer]

This PR adds a new configurable callback post_close_float (symmetrical to post_open_float), which can be assigned a function that takes the created tmp filepath as an argument.

The default implementation fs_unlinks the tempfile (automatically cleans it up). Introducing automatic file deletion is always a little scary, but since the current implementation overwrites the file content every time I can't think of a scenario where deleting it would be catastrophic.

Presumably this covers the intended changes from #31, albeit taking a slightly more general approach.

Test plan

Configured the hook to print and fs_unlink the temp file and verified it got printed and unlinked (only on close, not on save).

    post_close_float = function(tmp_filepath)
      print("got " .. tmp_filepath)
      vim.loop.fs_unlink(tmp_filepath)
    end,

And set the default implementation to delete the temp file.

[AckslD]

looks great @christoffer, one question I had is this should also be done one the BufHidden events?

[AckslD]

could this be moved before checking if nothing changed to avoid having this clause both here and below?

[christoffer]

I guess nothing relies on the file content in this callback, so that makes sense. Good call!

[christoffer]

looks great @christoffer, one question I had is this should also be done one the BufHidden events?

I'm not terribly familiar with the details of the vim callbacks, so I'll gladly accept any suggestions for which events it makes sense to unlink the original file.

Just browsing through the documentation quickly though, it seems like there's some configurable nuances related to buffer hiding which makes me reluctant to implement support for it. Do you know if buffer hidden is pracitcally useful for this scenario? I can't think of another way to have it trigger except switching buffer inside the float window, which seems like an edge case to support (user has autodelete hidden buffers enabled AND switches buffer inside the float window).

Intuitively, it feels like the WinClose is the right event (as you also suggested here. However, if there is something like a "BufDelete" or similar that is triggered for the appropriate buffer hiding behavior AND by the window close, then that would be ideal though.

Any suggestions or thoughts on how to decide what to support?

[christoffer]

Ah, nevermind, I just re-read the code and I assume you were discussing this behavior in particular, @AckslD?

That makes absolute sense, I'll revise the PR to hook into that part as well. Thanks!

[AckslD]

Yes indeed, I'm not sure this is the best way to handle it but my initial thinking with this was that if the user would change buffer in the open window I wanted some way to clean up the buffer etc. But maybe that's a bad idea and should be up to the user. For example if the user want to switch to some other buffer and back again. Happy to hear if you have some thoughts on this.

[christoffer]

Yes indeed, I'm not sure this is the best way to handle it but my initial thinking with this was that if the user would change buffer in the open window I wanted some way to clean up the buffer etc. But maybe that's a bad idea and should be up to the user. For example if the user want to switch to some other buffer and back again. Happy to hear if you have some thoughts on this.

Sorry for the slow response rate. These week end fiddle projects always slip down the priority list, you surely know how it is :)

Was thinking about this some more (and running in to roughly the same pattern in my own (personal) plugin). I landed in a conclusion that my personal idea of the cleanest way to deal with this would be this:

• Provide the user with a window containing some content
• Treat the closing of that window as the marker of when they're done, and trigger any cleanup

With this model, the BufHidden event seems a little eager. Your point of cleaning things up is sound, I think, and we still get the same effect by closing the buffer in the WinClosed instead.

One particular use case where I can see myself ending up in an unexpected state with the current BufHidden behavior is my (bad?) habit of using gd to jump to libraries to check the api/code. Editing some snippet of code and then gd to check the documentation and implemention of it would "break" the edit session since there's no buffer to <C-o> back to, right?

Either way, regardless of the decision on cleaning up buffers in BufHidden or not, I think that only cleaning up the temporary file on WinClosed makes sense. The window needs to be closed at some point, and it doesn't really matter if we're keeping the file around until then or not.

[AckslD]

you surely know how it is

I do indeed :)

I think I agree with you! Would you like to also remove the BufHidden autocmd? Could also be done in a separate PR.

[christoffer]

Closing this out, as it's unlikely I'll finalize this. Feel free to pick and chose any code from it if you feel it's useful. Thanks for the iterations!