Prompt before closing modified buffers

[keforbes]

When closing a buffer from the menu (Quit, Close, Split -> Close, Close Other Split(s)) Oni needs to prompt to save if the buffer is modified. For some operations (Close, Split -> Close) we just need to check if the current buffer is dirty but for other operations (Quit, Close Other Split(s)) we need to see if any buffer is dirty.

[keforbes]

@justinmk, in Vim, the standard way to see if any buffers are dirty would be to get all buffers and then iterate over them to see if the &modified flag is set. Would the same process be true in NeoVim or are there any special tricks I should be aware of?

[justinmk]

Vim has the 'confirm' option. Nvim may need to add a new UI event for this.

created an issue: https://github.com/neovim/neovim/issues/6201

[keforbes]

Thanks @justinmk! An event is what I'm looking for. If I run confirm close right now it'll display the prompt in NeoVim but I can't capture a signal to display a dialog. This means I can't provide a file dialog to enter a filename. The confirm command doesn't return until the prompt is acknowledged.

[justinmk]

@keforbes Yeah, instead of a 'confirm' event, we should just externalize the dialog.

[Bretley]

@keforbes are we looking to UI-ify this or just have the regular prompt for now

At the very least we could set confirm in the forced settings

[keforbes]

I'm torn between either fetching all buffers and checking their &modified flag to do this work manually or waiting for Neovim to implement this feature so we can do it the "right" way. The confirm option isn't a viable solution, it prompts within Neovim but doesn't notify Oni of the response so I can't pop-up a file dialog.

[bryphe]

At a high-level, it would be nice to have the following methods on NeovimInstance

• anyBuffersModified(): Promise<boolean> - returns true if a buffer has an unsaved edit, false otherwise
• saveAll(): Promise<void> - saves all active buffers

And then per-Buffer instance:

• isModified(): Promise<boolean>
• save(): Promise<boolean>

For our entry points to our close-UI, we can check anyBuffersModified and then pop-up our own UI. I think it's OK to do this in the interim - this is an experience that is really important in other editors, and how a user expects "Close/Quit" will behave - so without this, we have an experience that might mean someone loses their changes..

Also, the above methods will be useful in general for other functionality - so I believe it'd be reasonable to implement it that way.

[keforbes]

I was actually thinking we would need a getModifedBuffers() method which would return a list with buffer reference and current filename (if applicable) of every dirty buffer. If the resulting list is empty all is well, but if there are dirty buffers we wouldn't need to have follow-up method calls to fetch that information. We'd want to know the current filename to display it in the dialog and the associated buffer reference so we could perform the selected action (save or discard).

[bryphe]

This is a huge gap in our current story, and basic functionality that any text editor should have...

Ideally, we'd implement the following:

• Detection on close of an active / opened buffer
• A modal to ask for confirmation (it'd be great if this was our own UX, that implemented with 'sneak')

[justinmk]

ext_messages "msg_show" event in Nvim 0.4 should cover at least part of this use-case. Is there anything missing from it? @bryphe @Akin909 @CrossR @keforbes

[akinsho]

@justinmk it definitely sources a way to show the prompt which we can GUI-fy :+1:.

One thought I've had is that with that something like a confirm quit dialog you'd want to surface that quite differently to other ext_messages say we have a random plugin echo that can go near the bottom of a messages UI but you'd ideally want something as important as confirming close to appear more centrally. Classically a popup in the center of the screen.

As it stands currently you'd have to parse the returned messages for a matching string indicating a quit prompt message and then render that differently, since it's not a special case.

This is more just a UI concern that may be too specific to what I'm thinking for Oni 2 and tbh it's much more than we had when we opened this/haven't really gotten round to thinking about this problem yet for v2.

[justinmk]

One thought I've had is that with that something like a confirm quit dialog you'd want to surface that quite differently to other ext_messages say we have a random plugin echo that can go near the bottom of a messages UI but you'd ideally want something as important as confirming close to appear more centrally. Classically a popup in the center of the screen.

@Akin909 That's definitely already covered by ext_messages, because the msg_show event includes a "kind" field. Sometimes the "kind" field is empty for codepaths that we haven't annotated. https://github.com/neovim/neovim/pull/9993 adds support for more kinds.

I can imagine a future enhancement that exposes the "choices" for an input dialog, instead of sending the raw dialog text. But that is not planned for now.

[otto-dev]

A lot of my code just went poof