First draft of 'file_externally_changed' RFC

[Cogitri]

Relevant to #1126 and #1140

[cmyr]

So the way I would encourage thinking about this is as a sort of two-part question.

1. What is the behaviour we want, from the POV of the user? If a change happens on disk, do we want the user to know about it immediately, or only if they later try to save changes? In either case, what options would we like to present to the user?

2. What API should core provide to enable this behaviour? There are various ways we might support a given decision; for instance core could just notify the client of a change and then leave everything up to them, (for instance always taking a save message literally and overwriting whatever is on disk) or core could take a more active role, preventing saving when a file has changes but providing some API for resolving the conflict.

So given those questions, do you have an opinion of what approach might be best?

[Cogitri]

What is the behaviour we want, from the POV of the user?

As detailed in the RFC, there are two ways to handle it:

a) Notify immediately on change and reload or don't reload and overwrite
b) Don't notify immediately and confront the user about overwrite|save as|cancel during saving
c) Don't notify immediately and let the user save as normal. If the file can't be saved because it has been changed show user the previously mentioned save dialog.

What API should core provide to enable this behaviour?

IMHO we should go on as described in the RFC: Have a force param to keep the old behaviour (don't overwrite the file if the param is missing). I don't think we have to provide much of an API for resolving the conflicts (unless we want to show the user a diff, but I don't think we have to do that just now).

For a) and b) we only need the previously mentioned force param and a file_externally_changed notification (note: for b) the frontend will have to remember that the file has been changed).

For c) we need what a) needs and a way to communicate to the frontend that saving has failed (which might be a good idea in general)

[cmyr]

Will focus on the first part for now:

a) Notify immediately on change and reload or don't reload and overwrite

It's not clear what "don't reload and overwrite" means. Does this mean that if a file changes on disk we would immediately overwrite it, without notifying the user? I don't think this is what you mean, but it's not clear.

b) Don't notify immediately and confront the user about overwrite|save as|cancel during saving
c) Don't notify immediately and let the user save as normal. If the file can't be saved because it has been changed show user the previously mentioned save dialog.

What is the difference (in terms of the experience of the user) between these two options?

[Cogitri]

It's not clear what "don't reload and overwrite" means.

It means that the user doesn't reload (just keeps on editing) and on save overwrites the (changed) document.

What is the difference (in terms of the experience of the user) between these two options?

From my POV b) is better, since I'm (the user) am only asked one question, which has all the possible options for me. There's no need to press a button and wait around if that worked and if not press another button in a new dialog again like in c)

[cmyr]

It's not clear what "don't reload and overwrite" means.

It means that the user doesn't reload (just keeps on editing) and on save overwrites the (changed) document.

I think we can rule out this option, on the principal that we should not ever silently overwrite user data.

From my POV b) is better, since I'm (the user) am only asked one question, which has all the possible options for me. There's no need to press a button and wait around if that worked and if not press another button in a new dialog again like in c)

Core knows whether or not a change has occurred; there's no calculation involved. Given this, core should be able to respond instantly (in human time scales) and so immediately after 'save' you would see the dialog indicating that there had been a change. This is the sense in which I think these two are equivalent, from the perspective of the user? (They are subtly different for the frontend author, granted)

[Cogitri]

on the principal that we should not ever silently overwrite user data.

Maybe a combination of a+c would be best then? So notify the user on change and offer to reload and also notify on save that a change has occured?

Given this, core should be able to respond instantly (in human time scales) and so immediately after 'save' you would see the dialog indicating that there had been a change.

Ah, sorry, didn't do a good job communicating what I meant there. I mean that the user has to press 2 buttons instead of 1, read 2 dialogues instead of one (first "Do you want to save?" and then "Do you want to overwrite" and if he answers no to that has to click through the previous dialogue again). So it doesn't need a lot of computer time nor is it complex to implement for frontends, but rather it needs a lot of human time (compared to b).

[cmyr]

Ah, sorry, didn't do a good job communicating what I meant there. I mean that the user has to press 2 buttons instead of 1, read 2 dialogues instead of one (first "Do you want to save?" and then "Do you want to overwrite" and if he answers no to that has to click through the previous dialogue again). So it doesn't need a lot of computer time nor is it complex to implement for frontends, but rather it needs a lot of human time (compared to b).

I think I'm still confused, though. What would happen in c is that the user would send save (clicking from the menu, or using ctrlS / cmdS, saving would fail due to changes on disk, and the user would immediately see the dialog. I'm not sure where this other dialog is coming from?

[Cogitri]

Derp, I think my brain is a bit fried from cramming so much math into it, somehow I was sinking about the save dialog that's usually displayed when you attempt to close the editor but still have unsaved chsnges. You're right, usually there'd only be one dialog! :)

[cmyr]

Okay cool, glad we're on the same page.

[Cogitri]

Pushed a cleaned-up version.

[cmyr]

Okay, looking back over this I think it's a reasonable approach.

I think it would be nice to figure out exactly what the new RPCs will be / what their arguments are, so that could be something to add? Otherwise I think it's reasonable to start working on this.

One inline suggestion: it might be worth separately working on making save be a request RPC (that requires a response) so that we can consistently report success/failure.

[cmyr]

I think that probably save should report errors in general; I think it makes sense for save to become a request. Maybe this would be a good first step for this work? We would want to support the existing API fro the time being, but we could have both notification & request live alongside one another for a while.

[Cogitri]

but we could have both notification & request live alongside one another for a while.

Hm, can they also share a name?

[cmyr]

I'd have to check, but I think so; we separate notifications from requests early (based on the presence of the id field) so this should work. Let's try it out?

[Cogitri]

Alright, will adjust the RFC in a bit! :)

[cmyr]

S one last thought on this: normally (in other editors) when you get the notification that the file has changed, this doesn't happen in response to a save request.

So: I think there are different possible 'flows' in response to a notification of an external change, versus attempting to save. I would expect your options to in response to an external change to be "ignore/reload", and then if you ignore and attempt to save later that will still fail, unless you've included force.

More and more this makes me think that save should be a request that returns success or failure?

[Cogitri]

S one last thought on this: normally (in other editors) when you get the notification that the file has changed, this doesn't happen in response to a save request.

Well, there are different approaches to this (see the comparision under Force saving, VSCode/VS do it this way, Nano does it upon saving). I think to get a notification when the file has changed xi needs to be built with the notify feature enabled though?

I would expect your options to in response to an external change to be "ignore/reload", and then if you ignore and attempt to save later that will still fail, unless you've included force.

Not quite sure if I understand, you want the Frontend to remember if saving has failed for this to work?

More and more this makes me think that save should be a request that returns success or failure?

Very much so, at least Tau currently manually checks if the file is readable, which is should definitely be handled by Xi.

[cmyr]

S one last thought on this: normally (in other editors) when you get the notification that the file has changed, this doesn't happen in response to a save request.

Well, there are different approaches to this (see the comparision under Force saving, VSCode/VS do it this way, Nano does it upon saving). I think to get a notification when the file has changed xi needs to be built with the notify feature enabled though?

Yes, this will require the notify feature.

What I mean is: even when an editor does receive file change notifications (my vim does this) this is handled differently, with different options, then when a file change is detected only after attempting to save.

I would expect your options to in response to an external change to be "ignore/reload", and then if you ignore and attempt to save later that will still fail, unless you've included force.

Not quite sure if I understand, you want the Frontend to remember if saving has failed for this to work?

The frontend doesn't have to remember; when it goes to send save, this will fail, and the frontend can then prompt the user to force save or not.

More and more this makes me think that save should be a request that returns success or failure?

Very much so, at least Tau currently manually checks if the file is readable, which is should definitely be handled by Xi.

Yep, so if part of this change introduces a save request (we might need to call this save2 for the time being, and deprecate save) I think that makes sense.

[Cogitri]

What I mean is: even when an editor does receive file change notifications (my vim does this) this is handled differently, with different options, then when a file change is detected only after attempting to save.

Yes, but isn't that the job of the frontend? I'm a bit confused as to how this should be handled here.

Anyway, updated the RFC quite a bit.