Notepad3: Feature request: Undo should undo file reload/revert

[AlexVallat]

If the file is modified externally, and you reload or revert in order to pick up the changes (either automatically through the use of the File Change Notification functionality or manually through the Revert command), the change of text should go on the undo buffer.

So, I have a file with content "text". I edit it externally to be "text 2". In Notepad3 I reload the file to pick up the change to "text 2". If I now click Undo, the content should be "text", and the modified state be dirty. Redo would return the text to "text 2", and the modified state to clean.

Visual Studio, for example, uses this behaviour. It's quite useful when you find something has changed the file in a way you didn't want or expect, and you want to get the old contents back again.

[RaiKoHoff]

_Development Remarks:_

• this feature must be disabled for "file tail chasing mode" and "clipboard monitoring mode"

[RaiKoHoff]

Please test (experimental) development beta version _5.19.512.1698_XpErImEnTaL.
(Beta Channel: see #1129)

Known issue: redo an undo crossing file-revert event causes strange selection restore behavior.

[AlexVallat]

• [x] Revert with no existing undo stack: Passed
• [x] Revert with existing undo stack: Passed
• [x] Revert with redo stack: Passed
• [ ] Revert with no change to file: Failed. Undo command is available, it should not be
• [x] File Open/History/DragDrop: Passed. Undo not listed
• [ ] Modified state (* in title bar): Failed. On Revert, modified state should be clean (as the content matches what's on the filesystem). If Undone, it should be dirty (current behaviour: clean, if file was in clean state before revert)
• [ ] Monitoring Log (was Doc Tail Chasing): Failed. Probably due to the modified state issue. To reproduce problematic behaviour: open file, set File Change Notification to None. Enable Monitoring Log. Make an external change to the file. Note that file is updated, but modified state is set Dirty. Make a second external change to the file. Reload prompt is shown (even though file change notification is set to None).
• [x] File Change Notification Display message: Passed
• [ ] File Change Notification Auto-reload: Failed. Probably due to the modified state issue. The first external change is auto-reloaded, but subsequent changes result in the Reload prompt, as for Monitoring Log.

Apart from the modified state issue, this is looking good.

[RaiKoHoff]

@AlexVallat : Thank you for testing this feature. I will try to debug the "modified state issue" soon.

[RaiKoHoff]

Please test (experimental) development beta version _5.19.514.1702_RC.
(Beta Channel: see #1129)

Toggling "Monitoring Log" will clear Undo/Redo History (to avoid memory leakage on AutoReload).
Be aware of keeping complete File in undo-buffer (FileRevert) is a memory consuming operation ;-)

[AlexVallat]

• [x] Revert with no existing undo stack: Passed
• [x] Revert with existing undo stack: Passed
• [x] Revert with redo stack: Passed
• [ ] Revert with no change to file: Failed. Undo command is available, it should not be
• [x] File Open/History/DragDrop: Passed. Undo not listed
• [x] Modified state (* in title bar): Passed
• [x] Monitoring Log (was Doc Tail Chasing): Passed
• [x] File Change Notification Display message: Passed
• [x] File Change Notification Auto-reload: Passed

Seems to be fixed up well. I think Monitoring Log clearing Undo/Redo history is fine, I can't think of any workflow where Undo/Redo would make sense in that situation.

I don't know what sort of monster sized files you typically deal with, but I measured incremental memory usage at about 2x file byte size, which doesn't seem unreasonable to me. Considering the vast majority of text files I deal with are in the <1MB range, it would take a hell of a lot of refreshes before it starts to consume a worrying amount of memory!

Log files might be bigger, of course, but that's what the Monitoring Log option is for...

[RaiKoHoff]

@AlexVallat : Regarding the open point in your last comment:
Scintilla records the "change" (replacing current text with text from file) on its undo/redo stack.
Scintilla does not check, if the text is identically to the replaced text, only a "change text" is pushed to the stack. The same would beopen file - add a character - backspace , the text is the same as the text in the file, but the operations are available on the undo/redo stack.
In my opinion this is correct behavior (the save button should be disabled, cause it is correct, that the currently reverted text from file is the same as in the file - cleared "dirty" flag).

[AlexVallat]

I see. That makes sense, I guess, it would be a pain if you had to code in a check yourself for text equality before deciding to add to the undo stack or not. I don't agree with your example, though: add a character and backspace should be two entries in the undo stack, each one having caused a change!

[RaiKoHoff]

A little change to the open point would make a valid optimization request:

• [ ] Reverting a file, while current state is not "dirty" (just saved that file before, resp. no save needed),
  the undo operation should not be available.

This is easy to implement: Just ignoring the revert request :thinking: .

[RaiKoHoff]

Please test (experimental) development beta version _5.19.515.1703_RC.
(Beta Channel: see #1129)

[AlexVallat]

I'm not sure that's a good idea... Revert is often used to mean reload - even thought the state is not dirty, we know (or suspect) the file content has changed and want to see the new content. Ignoring the revert request would break this use case.

[RaiKoHoff]

The AutoReload and MessageReload on FileChangeNotification event still works,
just the manual Revert is skipped if the document is in "unchanged" state.
Maybe forced manual Revert should be enabled, if FileChangeNotification is OFF (None)?
Don't know, if this covers all use-cases :thinking:.

[AlexVallat]

Possibly. The only case I can think of that might not be covered there is that if FileChangeNotification is Display Message, but the user chooses not to reload in response to the message because they want to check something in the current text first. Then they are happy and want to reload, so hit F5, but the file would not be reloaded because according to the flag, it hasn't changed.

Just generally risky to prevent the user from reloading if they've explicitly given the reload command, in my opinion.

[RaiKoHoff]

Okay, agreed - will revert the last change - this means, we have to live with the "undo" availability on FileRevert. even if the revert does not change the document.

[hpwamr]

Hello @AlexVallat ,
As far as I'm concerned, I think you (requester) can close this issue...