RickStrahl / MarkdownMonster

An extensible Markdown Editor, Viewer and Weblog Publisher for Windows
https://markdownmonster.west-wind.com
Other
1.56k stars 234 forks source link

Feature Request: Add `Reload` command #1099

Closed jbridgy closed 2 months ago

jbridgy commented 3 months ago

MM often does not detect changes to files of open documents by external apps upon regaining focus. Therefore I suggest to add a Reload command as it exists in many other editors, allowing users to manually refresh the current document. If the document has been modified with MM then theReload command should prompt the user with a warning that any unsaved changes will be lost, similarly to the Close command.

RickStrahl commented 3 months ago

If the document on disk has changed and you don't have changes it'll automatically reload the content. IOW, it reloads silently, unless you have pending changes in which cases it does nothing - until you save.

If you want to reload and you think there are changes on disk, just save - you'll be prompted to use theirs or yours or compare and merge.

Checking behavior in VS Code I see similar behavior - if changes occur if the document has been changed on disk it'll notify you on save by asking to compare and overwrite which is similar to what MM does (except you get more choices).

jbridgy commented 3 months ago

In the meantime I realized that F5 already provides the suggested Reload command, including the prompt, when the current document has been changed with MM.

image

So my request boils down to make F5 visible as default key binding for a Reload or Refresh command in the File menu.

If the document on disk has changed and you don't have changes it'll automatically reload the content. IOW, it reloads silently, unless you have pending changes in which cases it does nothing - until you save.

In my environment this works fine with some externals programs, especially with my favorite source code editor. However, it does not work with some others, especially not with my favorite compare and merge tool.

If you want to reload and you think there are changes on disk, just save - you'll be prompted to use theirs or yours or compare and merge.

MM has this feature for quite some time, as far as I remember, but until recently the Compare function did not work at all with my compare and merge tool. Therefore I disregarded it. Just now I have given it a try again with the following results:

RickStrahl commented 3 months ago

Hmmm... I can duplicate teh issue with 'Use Theirs' although a brief look at the code makes me wonder why that is. Use Theirs should just reload the document from disk even if there are changes and that's what the code is doing. I'll have to check...

As to Compare - you can configure your Diff tool. i use BeyondCompare:

image

executable has to be able to accept two filenames for parameters (which any diff tool should support).

MM also hunts for a few diff tools by default on your system if they are installed in their default locations.

RickStrahl commented 3 months ago

Ok I fixed the issue with the 'Use Theirs` not reloading.

Also modified the Context Menu to add the Reload from disk option and now display a less busy context menu if the Window is small. If window.height > 1100 then continue to show the old full menu.

image

All the hidden options are available on the Tab context menu but the editor menu is just too big if there's not a lot of screen real estate.

jbridgy commented 3 months ago

Surely it is a progress that MM's UI displays the standard functionality of F5 (= Reload) somewhere now. Also it is fine that Use Theirs after Ctrl+S (= Save) works now when the document has been modified externally. Nevertheless I suggest the following two improvements:

  1. The Reload command mainly belongs to the File menu where it appears in many other apps. It need not appear also in the context menu of the document or its tab. Anyway I think the context menu of the document should not depend on the size of the window. The File menu may depend on the screen size, not on the window size. Of course the screen size may be small as well, but this could matter only because MM follows the terrible fashion of newer UIs to waste vertical space. The following example shows that the vertical spacing of the LibreOffice menus is more reasonable.

    File Menu of Markdown Monster v3.2.9.2 File Menu of LibreOffice Writer v24.2.2.2
    grafik grafik
  2. Currently the following dialog occurs if the user applies the Save command (= Ctrl+S) and the document has been modified externally, regardless whether the document has been modified with MM or not.

    grafik

    I think that Ctrl+S should behave like F5 if the document has not been modified with MM. This would be consistent with the behavior when MM detects an external change automatically (without being triggered by Ctrl+S) and the document has not been modified internally.

Furthermore Compare after Ctrl+S fails in the following edge case:

These steps result in the following error:

grafik

RickStrahl commented 3 months ago

1 - not adding a Reload button to the file menu. It's enough of an edge case as is. as to menu spacing. The horrible practice is to have menus with a million things on it :smile:

2 - You can't be serious. The files are different and the external file most likely has changed. The dialog lets you know that the files are different since the last time MM loaded or saved. There are scenarios where you might have saved and the external file update occurs but doesn't notify for various reasons. You might have a change and undo it (now unchanged again, but you the external file was saved in the meantime).

Again these are all edge cases at best and being extra careful to avoid overwriting is more important than a little bit of potential convenience that may result in overwritten data. This logic for this is complex enough with all the scenarios and the fact that notifications are sometimes lost by Windows so you may end up without a notification when you should get some so being sure that the files you are saving are different between the last CRC check is the safe route.

RickStrahl commented 3 months ago

As to the odd crash - I was able to duplicate that once, but not since. Not quite sure what could be happening there other than the tool keeping the file open and in some weird state. What's odd is that the WebView is failing - I would expect that to fail when the file is loaded, unless the file result is null perhaps, but that is being checked for.

What tool do you use? (I use BeyondCompare here)

jbridgy commented 3 months ago

1 - not adding a Reload button to the file menu. It's enough of an edge case as is. as to menu spacing. The horrible practice is to have menus with a million things on it 😄

I understand your reluctance to devote more time to what seems like a minor issue. If that's the case, simply acknowledging it would suffice. However, it seems you're mixing up different aspects in your argument, when the situation is actually quite straightforward:

  1. The Reload command is customarily located in the File menu, a logical choice also adopted by numerous other applications.

  2. You've placed the Reload command in the document's context menu, which is inconsistent in the first place for several reasons. Instead of letting it be, you unnecessarily complicate the matter by making the context menu vary with the window's size, branding this as a novel feature. In reality, no user would expect such behavior.

  3. If you have concern about menus being too lengthy then the most straightforward remedy is to reduce the excessive vertical spacing, as demonstrated by LibreOffice. This is a basic principle of user interface design, rather than an edge case.

2 - You can't be serious. The files are different and the external file most likely has changed. The dialog lets you know that the files are different since the last time MM loaded or saved. There are scenarios where you might have saved and the external file update occurs but doesn't notify for various reasons. You might have a change and undo it (now unchanged again, but you the external file was saved in the meantime).

Again these are all edge cases at best and being extra careful to avoid overwriting is more important than a little bit of potential convenience that may result in overwritten data. This logic for this is complex enough with all the scenarios and the fact that notifications are sometimes lost by Windows so you may end up without a notification when you should get some so being sure that the files you are saving are different between the last CRC check is the safe route.

Your logic eludes me. When a user triggers Reload and the document has not been modified internally, then they simply want to ensure that the document on screen matches the one on disk. They press F5 especially because they expect that the document may have been modified externally, otherwise there is no reason to press F5. Thus, a dialog box with a Compare button becomes an unnecessary hindrance. There is one very rare case that might justify this dialog, namely when the external program inadvertently has overwritten the last changes saved by MM. However, this is a symmetric problem. So it's up to the user to hit F5 in the external program before they start making changes with it. This leads to another issue: Why do other editors detect external changes more reliably than MM?

jbridgy commented 3 months ago

As to the odd crash - I was able to duplicate that once, but not since. Not quite sure what could be happening there other than the tool keeping the file open and in some weird state. What's odd is that the WebView is failing - I would expect that to fail when the file is loaded, unless the file result is null perhaps, but that is being checked for.

What tool do you use? (I use BeyondCompare here)

I also use BeyondCompare.

RickStrahl commented 3 months ago

1 - LOL! Done with this one... we can quibble over semantics but at the end of the day that's my call. If you want to keep this up start a new Issue please.

2 - I don't follow your scenario. It's really hard to have a scenario where you have to reload in the first place - any external changes are immediately propagated into the document if there are no changes. It's hard for me to even set up a repro case because as soon as I change the file it updates in MM. That's my point. There should very rarely be a reason to reload because the document is automatically updated (unless the update events are not firing which is very rare but does happen). if one does get by and doesn't update the document.

You are conflating different operations. Reload only shows a dialog that reminds you that you have changes and gives you a chance to abort. Save gives you the compare dialog...

RickStrahl commented 3 months ago

Just to be clear so that we're talking about the same thing:

For both scenarios I use VS Code with the same file open as in MM

1) Reload with changes in MM

This is correct behavior.

2) Reload with out Changes in MM

This scenario very hard to even simulate because if I don't have changes in MM the document automatically updates when I save changes in VS Code. I have to fake it by typing into MM (dirty doc), saving in VS Code, then undoing the changes in MM. Now I have a dirty doc on disk and clean doc in MM.

This is also what I expect.

Note If the document is really unchanged the green Save button in the toolbar should be disabled. If it's not then the doc has some change (whitespace or encoding or a previous document update from a change.

3) Save Without Changes

4) Save With Changes File has not changed

5) Save with Changes and File has changed externally

RickStrahl commented 3 months ago

FWIW - I took a look at the menu styling and removed a little of the padding to save some space. Nothing as drastic as the crammed LibreOffice menu but it still is more compact.

Thanks for the inspiration...

jbridgy commented 3 months ago

Firstly I would like to restate the fact which triggered this thread:

MM does not detect external changes reliably, unlike other editors such as Visual Studio, VS Code or Notepad++. Often, this misbehavior requires manually reloading the current document.

Such a situation can reproduced easily with Beyond Compare as follows:

  1. Open SomeText.md with MM and some other editors.
  2. Change and save SomeText.md with Beyond Compare. For example restore a line from an old version of SomeText.md.

All editors except MM almost always (presumably >99.9%) detect the change immediately, at least on my computer. However, this is another issue.

I have repeated all your tests and more for Reload (F5) and Save (Ctrl+S) with MM 3.2.9.3. Here are the results.

BTW: Do you have an infrastructure for automated GUI tests?

Test Cases for Reload (F5)

Case Internal change
unsaved
External change
on disk
Passed Remarks
1 no no yes needless operation but harmless
2 no yes yes supposed to happen automatically
3 yes no yes shows dialog "Refresh Document"
4 yes yes yes? shows dialog "Refresh Document"

You wrote that case 2 is very hard to even simulate. It can be reproduced with Beyond Compare as mentioned above.

Sorry, I reported case 2 wrongly in the preceding post. Unfortunately, I conflated Reload case 2 with Save case 2, which was the one I actually wanted to restate. See also next section.

In case 4 the dialog "Document Save: File Conflicts" instead of dialog "Refresh Document" may be a preferable option. Now the user has to cancel the Reload command and then apply the Save command in order to preserve both internal and external changes.

Test Cases for Save (Ctrl+S)

Case Internal change
unsaved
External change
on disk
Passed Remarks
1 no no yes? only with Ctrl+S: needless operation; changes Modified timestamp
2 no yes yes?? only with Ctrl+S: shows dialog "Document Save: File Conflicts"
3 yes no yes normal case
4 yes yes yes shows dialog "Document Save: File Conflicts"

Again, case 2 can be reproduced with Beyond Compare as mentioned above.

Actually Ctrl+S should be disabled in cases 1 and 2. However, if Ctrl+S is not disabled then it should behave like Reload in case 2.

Ctrl+S always saves the current file to disk, regardless whether the save button is enabled (green) or not. In my opinion this is a bad behavior. The Modified timestamp of a file should never be changed needlessly. Some apps respect this convention (for example Word and Notepad++). MM is ambivalent. The Save command in the File menu and the corresponding image button respect it, but Ctrl+S does not.

RickStrahl commented 3 months ago

Hmmm... so this is interesting. I can corroborate what you're seeing with BeyondCompare. However, if I use VS Code to make changes to the document and save I see the change immediately.

It looks like for some reason the save operation in BeyondCompare is not triggering the file change event the same way as the save in VS Code or Notepad does (using the Windows FileWatcher).

I think that may be part of the reason we see many of the issues you're describing.

However, if I then save the file change is detected and I get the dialog that prompts me to choose Mine or Theirs which is as expected given that the file was not updated in the editor.

As to Ctrl-S always saving even if there are no changes - given that there may always be situations where the document may get out of sync with what's on disk I believe the safe thing to do is save when asked. The above example exemplifies that very issue - there may be a change that has occurred on disk, but you don't know about it but yet you still want to save your current work even though nothing has changed. I think the simple solution to that problem is, if you don't want the file date to change, don't save.

jbridgy commented 3 months ago

It looks like for some reason the save operation in BeyondCompare is not triggering the file change event the same way as the save in VS Code or Notepad does (using the Windows FileWatcher).

I suggest comparing MM's code for the detection of external file changes very closely with the code in VS Code or Notepad++ (both open source). Probably the other editors do it differently.

As to Ctrl-S always saving even if there are no changes - given that there may always be situations where the document may get out of sync with what's on disk I believe the safe thing to do is save when asked. The above example exemplifies that very issue - there may be a change that has occurred on disk, but you don't know about it but yet you still want to save your current work even though nothing has changed. I think the simple solution to that problem is, if you don't want the file date to change, don't save.

Your logic is elusive to me. Halfway through, MM has already adopted the approach used by Word and Notepad++. For consistency, just ignore Ctrl+S when the Save command and the corresponding button are disabled. Furthermore, it is desirable not to have to pay attention to the modified status, because the frequently needed keyboard shortcut Ctrl+S is often pressed unconsciously out of habit, following the motto: Save early, save often. Nevertheless the Modified timestamp reliably tells when the last change was made.

RickStrahl commented 3 months ago

Visual Studio Code behaves exactly the same as MM does. That is the model I'm using.

As to Ctrl-S always saving even if there are no changes - given that there may always be situations where the document may get out of sync with what's on disk I believe the safe thing to do is save when asked. The above example exemplifies that very issue - there may be a change that has occurred on disk, but you don't know about it but yet you still want to save your current work even though nothing has changed. I think the simple solution to that problem is, if you don't want the file date to change, don't save.

We've been through this. If you save and there are changes in the file you'll get the Use Yours/theirs/compare dialog so you get to make a decision what you want to do. This is the safe thing to do, and... let's be honest that should be an increasingly rare situation.

There's nothing wrong with the save operation itself and there won't be any changes to that.

What has a potential issue is that some disk changes are not being detected or are not updating the document with the changes, when the document is unchanged, and I'm looking into that.

Specifically the issue with Beyond Compare which I suspect is writing out the updated file with missing some flag that I'm (or the FileWatcher) looking for modified state detection.

RickStrahl commented 3 months ago

Alright there's some progress on the Beyond Compare issue which was caused by two things:

But there are other issues related to managing the compare and passing the results back into MM.

Started a new issue #1100