Make session missing files remembered

[donho]

From #14168
Note that an option to activated this feature (remember all session files) has not been added yet.

Fix #12079, fix #12744, fix #13696

[donho]

@Yaron10
I've just done the last effort to synchronize the execution of YesAll/NoAll dialog (message DOC_DELETED sent by PostMessage), the effort is in vain :(
Just like you, I prefer the solution YesAll/NoAll dialog. However it introduces a bug in some circumstance, I think I have no choice but opt for the solution of opening all with an alert to user.

You can always create an issue of feature request for YesAll/NoAll dialog, so someone more competent than I could implement this feature.

Thank you for your understanding and for your participation of this issue.

[Yaron10]

@donho,

Thank you for your hard work.

One last clarification:

If the files were truly permanently gone (deleted intentionally), that behavior makes sense.

@pryrt in #12079 (comment).

STR:
Intentionally delete 50 files from your last session.
Open NPP.

Result:
NPP opens 50 place-holders without asking the user.

This a major bug IMO. Possibly even worse than the current (master) behavior.

***

And one last question:
Would a "Yes/No" message be OK?
Something like:

Notepad++ can not open some files from your last Session.
Would you like to create place-holders for them? 

Ideally: open NPP, display the tab-bar (and tabs), prompt and run the place-holders mechanism from there.

---

A short code-snippet which can be added to the end of Notepad_plus::loadSession().
(sessionFileName should be obtained there).

	if (!allSessionFilesLoaded)
	{
		if (sessionFileName == nullptr)		// session.xml.
		{
			_nativeLangSpeaker.messageBox("AutoSessionFilesError",
				_pPublicInterface->getHSelf(),
				L"Notepad++ can not open some file(s) from your last Session.\n\nThe Session file \"session.xml\" will be modified on exit!\nYou can backup that file NOW.",
				L"Opening Session Files Error",
				MB_OK | MB_ICONWARNING);
		}
		else		// Manually-created session file (File -> Save Session).
		{
			_nativeLangSpeaker.messageBox("UserSessionFilesError",
				_pPublicInterface->getHSelf(),
				L"Notepad++ can not open some file(s) included in the following Session file:\n\n\"$STR_REPLACE$\"",
				L"Opening Session Files Error",
				MB_OK | MB_ICONWARNING,
				0,
				sessionFileName);
		}
	}

---

Anyway, the place-holders can be as useful in user-sessions as they are in session.xml.
The only difference is whether the session file should be auto-modified by NPP.

[donho]

@Yaron10

NPP opens 50 place-holders without asking the user.
This a major bug IMO. Possibly even worse than the current (master) behavior.

It's not a bug for me. Users have the choice to not enable this feature, but if they enable it, they know what they are doing.

And one last question:
Would a "Yes/No" message be OK?

Though I don't consider it (opening all place-holders if this option is enabled) as a bug, I prefer still that users can choose to open them or not when the option is enabled. So I did the last try last night, and it works quite good. The final solution as you suggested has been committed into this PR. For unknown reason, the CI doesn't work, you can check the PR and compile it locally if you want.

Anyway, the place-holders can be as useful in user-sessions as they are in session.xml.
The only difference is whether the session file should be auto-modified by NPP.

All the work done in v8.5.8 for user created session is just for distinguishing user-session from session.xml. As a result they won't have the same behaviour. In the latest commit of the PR, the place-holder feature has been removed.

[Yaron10]

@donho,

I prefer still that users can choose to open them or not when the option is enabled. So I did the last try last night, and it works quite good.

Great! 👍
I'd like to test it more thoroughly, but - so far - it seems to be working like a charm.
Thank you again for your work. I appreciate it.

In the latest commit of the PR, the place-holder feature has been removed.

Could you please explain to me why the place-holders shouldn't work for user-sessions as well?
It might require some extra thinking and work, but isn't it worth while? Why not make it perfect?

---

https://ci.appveyor.com/project/donho/notepad-plus-plus/builds/48319984

[donho]

@Yaron10

I'd like to test it more thoroughly, but - so far - it seems to be working like a charm.

Nice to learn about that :)

Could you please explain to me why the place-holders shouldn't work for user-sessions as well?
It might require some extra thinking and work, but isn't it worth while? Why not make it perfect?

The logic is simple and it came from you: user-session should not be modified by Notepad++ for users.
I realized that indeed it shouldn't because there's Save session... command for modify user-session.
Based on this rule, place-holder has no sense for user-session because it will create the confusion. If a user closes 1 or some place-holder(s), there could be 2 behaviours:

1. Modify user-session then in this case we violate the base rule and Save session... command become useless.
2. Not modify user-session then user won't understand why these place-holders are there.

Both brhaviours are not consist for users, so that's the reason to not have place-holders in user-session.

[Yaron10]

@donho,

Thank you for the explanation.

place-holder has no sense for user-session

The advantage with "place-holder" is, when user opens Notepad++, and realizes he/she forgets to mount a volume / insert a USB (because of the prompt), user can always insert the USB or mount the network driver, then switch to any place holder, the prompt of reloading message launch so he/she can get the files back without relaunching Notepad++. (#12079 (comment)).

This is valid for user-sessions as well, isn't it?

because it will create the confusion

Why? I think it's quite consistent.
The user should know that NPP does not modify user-sessions. Period.
Keep the place-holders, close them. - NPP shouldn't care.

Yes, you'd need to use two different messages (one for session.xml and another for user-sessions), but isn't it worth it?

---

I haven't "learned" the code. Still, allow me to ask:
I see the advantage in handling the message(s) and result in FileManager::newPlaceholderDocument().
But FileManager::newPlaceholderDocument() could be used (some day in the far future) for missing files in "Recently Closed Files".
Also - if you agree that place-holders are useful for user-sessions, it might be easier to handle the message(s) and result in Notepad_plus::loadSession().

[xomx]

@donho @Yaron10
So far everything works for me ok in my tests of this PR.
Thank you for your time & effort.

[Yaron10]

I haven't actually tested it.
If res == IDNO - is there a reason to continue here and keep calling this function in loadSession()?

[donho]

Well caught!
Fixed here: ccf6f75

[Yaron10]

@xomx,

Thank you, @donho & @molsonkiko.

Do you think that implementing place-holders for user-sessions might be confusing?
IMO, not doing it would be confusing & unexpected.

[Yaron10]

IMO, not doing it would be confusing & unexpected.

It would actually make it much worse compared to current master.
The user sees the message for missing files in session.xml (-> "Great feature! NPP notifies me if files are missing.").
The user opens a user-session containing some missing files (-> "I haven't been notified. NO MISSING FILES.").

[donho]

@Yaron10

I see the advantage in handling the message(s) and result in FileManager::newPlaceholderDocument().
But FileManager::newPlaceholderDocument() could be used (some day in the far future) for missing files in "Recently Closed Files".
Also - if you agree that place-holders are useful for user-sessions, it might be easier to handle the message(s) and result in Notepad_plus::loadSession().

Done in the latest commit.
Could you test it to be sure there's no new bug and either regression?

[Yaron10]

@donho,

Thank you for the recent changes and fixes.

I'm not sure which commit should be tested.
Also - it's currently "59 files changed", and I'd like to get some idea of this PR code-changes.

I'll redo another PR, so for the moment we can ignore it.

Do you mean closing this PR and opening a new one?