Make session inaccessible files remembered (part 2/2) #14252

donho · 2023-10-20T20:20:03Z

Based from #14168
Continue with #14202

Fix #12079, fix #12744, fix #13696

Fix issue #12079 If a file was not backed up when a session closed, previously when the session was reloaded, those files would simply disappear and the user could not choose to keep them Now an empty placeholder buffer will be created for any files without backup with a filename that doesn't exist. This placeholder can be reloaded from hard drive if the file it corresponds to is recreated, and its placeholder status will persist as long as it stays empty. The handling of backed-up files that don't exist will remain the same.

…b.com/molsonkiko/notepad-plus-plus into molsonkiko-fix_session_missing_files_remembering

Based from #14168 Fix #12079, fix #12744, fix #13696

donho · 2023-10-20T20:22:51Z

@Yaron10

Done, please check it.

Yaron10 · 2023-10-20T22:32:12Z

@donho,

👍
Thanks again for your work and patience.

I'll continue testing it tomorrow.
So far:

STR:
Open file A.
File -> Save Session: save it as SessionA.ses.
Close NPP.
Delete file A.
Open SessionA.ses from File-Explorer.
Click "Yes" in the two messages.

Result:
Two A tabs in the same view.

STR:
Create two user-sessions.
Delete some file(s) from one session file.
Select the two user-sessions in File-Explorer and open them in one go.

Result:
You can not know which session file has missing files.

How about replacing bool loadSession(Session & session, bool isSnapshotMode = false, bool isUserCreatedSession = false); with bool loadSession(Session & session, bool isSnapshotMode = false, const TCHAR *sessionFileName = nullptr);?

- You could then display the user-session file-name in the message.

"Open Session in New Instance (and save on exit)" complicates the implementation/messages.

notepad-plus-plus/PowerEditor/src/Notepad_plus.cpp

Line 6487 in c8fafcd

else

Is that still needed?

ref: #14252 (comment)

donho · 2023-10-21T02:25:17Z

@Yaron10

STR:
Open file A.
File -> Save Session: save it as SessionA.ses.
Close NPP.
Delete file A.
Open SessionA.ses from File-Explorer.
Click "Yes" in the two messages.

Result:
Two A tabs in the same view.

Fixed.

"Open Session in New Instance (and save on exit)" complicates the implementation/messages.

Could you elaborate it?

Is that still needed?

Simplified.

Please check the latest commit.

PowerEditor/installer/nativeLang/french.xml

Yaron10 · 2023-10-21T15:06:59Z

@donho,

Fixed.

👍

But this creates another issue.
STR:
Open file A.
Save session as SesA.ses.
Open file B.
Move B to the other view.
Save session as SesAB.ses.
Move B to the other view (-> A and B in one view).
Close NPP.
Delete A and B.
Select the two user-sessions in File-Explorer and open them in one go.

Result:
No B in MAIN_VIEW.

How about replacing bool loadSession...

I understand you don't like it.

"Open Session in New Instance (and save on exit)" complicates the implementation/messages.

I'm referring to the potential confusion you mentioned in #14202 (comment).

The messages should indeed make it clear.

User-sessions.

Some files from your manually-saved session are inaccessible. They can be opened as empty and read-only place-holders.
Would you like to create those place-holders?

NOTE: Choosing not to create the place-holders or closing them later, your manually-saved session will NOT be modified on exit.

session.xml.

Some files from your past session are inaccessible. They can be opened as empty and read-only place-holders.
Would you like to create those place-holders?

NOTE: Choosing not to create the place-holders or closing them later, your session WILL BE MODIFIED ON EXIT!
You can backup your \"session.xml\" now.

"Open Session in New Instance (and save on exit)" will indeed cause confusion.
Regardless, I don't understand why you should make an exception in this case and modify the user-session.

Yaron10 · 2023-10-21T15:40:25Z

But this creates another issue.
STR:

You get the same result without deleting the files. - Probably not related to this PR.
It should be further investigated with current master and older NPP versions.

donho · 2023-10-21T15:53:47Z

But this creates another issue.
STR: Open file A. Save session as SesA.ses. Open file B. Move B to the other view. Save session as SesAB.ses. Move B to the other view (-> A and B in one view). Close NPP. Delete A and B. Select the two user-sessions in File-Explorer and open them in one go.

Result: No B in MAIN_VIEW.

Cannot reproduce it:

How about replacing bool loadSession...
I understand you don't like it.

Implementing...

donho · 2023-10-21T16:43:33Z

@Yaron10

The messages should indeed make it clear.

Done. However, according google, "place-holder" should be "placeholder", so I maintain "placeholder".

"Open Session in New Instance (and save on exit)" will indeed cause confusion.
Regardless, I don't understand why you should make an exception in this case and modify the user-session.

"Open Session in New Instance (and save on exit)" was a frequent-request feature which was implemented quite long time ago, so I believe a lot of users depend on this feature.

You can check the latest commit in which the messages have been refined (with the user session file name).

Yaron10 · 2023-10-21T19:11:40Z

@donho,

It seems to work like a charm. 👍
I hope we haven't missed some corner cases.

Thanks again.

Done. However, according google, "place-holder" should be "placeholder", so I maintain "placeholder".

👍
We need a native English speaker.

They can be opened as empty and read-only placeholders.

Instead of

They can be opened as empty and read-only files as placeholders.

?

And \n before You can backup?

"Open Session in New Instance (and save on exit)" was a frequent-request feature which was implemented quite long time ago,

Do you mean the (and save on exit) part was a frequent-request feature?

You get the same result without deleting the files. - Probably not related to this PR.
It should be further investigated with current master and older NPP versions.

Could you please try the exact steps?

STR:
Open file A.
Save session as SesA.ses.
Open file B.
Move B to the other view.
Save session as SesAB.ses.
Move B to the other view (-> A and B in one view on exit).
Close NPP.
Select the two user-sessions in File-Explorer and open them in one go.

Result:
No B in MAIN_VIEW.

Yaron10 · 2023-10-21T22:43:39Z

@alankilborn,

Could you please test this PR?
Thank you.

pryrt · 2023-10-21T23:28:03Z

Regarding "place-holder" vs "placeholder", I definitely agree that "placeholder" is the right choice.

@alankilborn suggested I chime in, since I was the one who submitted #12079. Based on my experiments, this meets my expectations for reasonable behavior regarding the session when a file/directory/mounted-drive goes temporarily missing, and I would be happy if this is the version that goes "live".

Thank you for all the effort that the whole team put into going through various edge cases, and all the back-and-forth that occurred. The end result shows the benefit of getting a lot of different perspectives on this issue.

Yaron10 · 2023-10-21T23:45:01Z

@pryrt,

Thank you for submitting the issue and testing this PR.

@alankilborn,

That was a nice dodge. :)

donho · 2023-10-22T00:29:14Z

@Yaron10
I do believe "They can be opened as empty and read-only files as placeholders" since it's chatgpt has suggested me :)
However, I think to use "documents" instead of "files" is even better: "They can be opened as empty and read-only documents as placeholders".

Do you mean the (and save on exit) part was a frequent-request feature?

Yes.

Could you please try the exact steps?

Cannot always reproduce it.

Note that I copied new compilerf bin to replace Path : C:\Program Files\Notepad++\notepad++.exe and I used Edit with Notepad++ to open both session A.sess & AB.sess.

Yaron10 · 2023-10-22T12:56:23Z

@donho,

I do believe "They can be opened as...

👍

and I used Edit with Notepad++ to open both session A.sess & AB.sess.

I'm using NPP portable, and I don't have that entry in the context menu.

Both files are selected, but does it make a difference if you right-click on A.sess or AB.sess?
And if you use Open?

I'm using QTTabBar and OldNewExplorer on Windows 10.

On using Open in the bar

I always get a wrong result: No B in MAIN_VIEW.

On using Open in the context menu, it depends on the file I right-click.

#14161 might be related.

donho · 2023-10-22T17:32:56Z

@Yaron10
Thank you for the more detail info

Both files are selected, but does it make a difference if you right-click on A.sess or AB.sess?

I suppose by "right-click" you means Edit with Notepad++ of context menu under Explorer which comes Windows 10/11.
When you right-click on A.sess or AB.sess and run Edit with Notepad++ from the context menu, NppShell.dll runs command notepad++.exe "A.sess" "AB.sess" - both session files are opened in one call, and session files are opened one after another.

In this scenario, it works fin, as expected.

And if you use Open?

Always under Explorer which comes Windows 10/11, when you select 2 files and use Open command from the context menu, or type ENTER key to open 2 files: the system run notepad++.exe "A.sess" then notepad++.exe "AB.sess" (or in inverse order, it depends in which order you select the 2 files).

In this scenario, I can reproduce the problem, not exactly the same result though (but even worse):

I will see what I can do about it.

donho · 2023-10-22T19:25:00Z

I will see what I can do about it.

The test results (with open) are different on mono-instance - multi-instance works perfectly - and it's surely due to the race condition: System launches several Notepad++ commands simultaneously and all commands lead to one instance for the execution in a random order.
Unfortunately I've no solution so far. And at the same time, this issue is related to this feature but it's not a regression due to this PR, so I will merge this PR.

Yaron10 · 2023-10-22T20:39:16Z

@donho,

In this scenario, I can reproduce the problem, not exactly the same result though (but even worse):

Yes, I encountered a similar case after posting my STR.

I will see what I can do about it.

Thank you for looking into it and good luck.
Could you please open an issue? - Your specs are (obviously) much more common.

And at the same time, this issue is related to this feature but it's not a regression due to this PR, so I will merge this PR.

👍

donho · 2023-10-23T13:08:14Z

@Yaron10

Could you please open an issue? - Your specs are (obviously) much more common.

It's indeed the same issue of #14161:
I tested with 1 modified untitled text file in the session. With no Notepad++ open, I select 4 text files and launch Open via Explorer.
After several tests I cannot reproduce the lost of the modified untitled document in the current session as @OsakaWebbie has experienced. However, I have reproduced once (among 6-8 times) that 1 of these 4 files was missing - it's what it has happened to you with session files.
As I have explain here & there, system launches several commands to call Notepad++, that will raise the race condition (probably it's better explained by portableapps).

As a result, I won't open a similar issue as #14161, and this issue won't be fixed and probably can't be fixed.
The best practice is using the "Edit with Notepad++" of context menu or a one command with all files as arguments in command line.

Yaron10 · 2023-10-23T13:53:29Z

@donho,

First, I'd like to emphasize again that I was initially wrong: the issue is not related to this PR, and can be reproduced in v8.5.7 as well.

In this scenario, I can reproduce the problem, not exactly the same result though (but even worse)

I understand your explanation and recommendation.
But ending up with two identical-path tabs in the same view is a major problem, isn't it?
(And again, it can happen with accessible files as well).

The question is if NPP should (and could) make sure this is not the end result.
If no other solution is viable - how about checking if an identical-path already exists in the view, and prevent opening another one?

Yaron10 · 2023-10-23T14:33:55Z

Alternatively, and as it's a rare case, it can be labeled as a "known issue".
The best applications have them. :)

donho · 2023-10-23T14:53:52Z

If no other solution is viable - how about checking if an identical-path already exists in the view, and prevent opening another one?

It's already done in the code.
The problem of race condition is the same resource is accessed at the same time, that makes the identical-path checking for 2 threads or 2 processes have the same result. That's the reason that the identical-path checking not working in my test.

Alternatively, and as it's a rare case, it can be labeled as a "known issue".

Why not? I can always labeled #14161 as "known issue". But what's the advantage?

Yaron10 · 2023-10-23T15:07:34Z

Thank you for the explanation.

I can always labeled #14161 as "known issue".

That issue is only reproducible with PortableApps.
The cases we've encountered here are valid in common regular setups.

But what's the advantage?

So that we at least have an open issue and not just some comments in a closed PR.
As you understand. :)

molsonkiko and others added 3 commits September 21, 2023 08:18

Merge branch 'fix_session_missing_files_remembering' of https://githu…

d39758a

…b.com/molsonkiko/notepad-plus-plus into molsonkiko-fix_session_missing_files_remembering

Make past session inaccessible files remembered (Part 2)

c8fafcd

Based from #14168 Fix #12079, fix #12744, fix #13696

Fix same placeholder repeated bug & simpfy the code

27e8a63

ref: #14252 (comment)

Mateos81 reviewed Oct 21, 2023

View reviewed changes

PowerEditor/installer/nativeLang/french.xml Outdated Show resolved Hide resolved

Mateos81 reviewed Oct 21, 2023

View reviewed changes

PowerEditor/installer/nativeLang/french.xml Outdated Show resolved Hide resolved

Refine the messages & fix typos in french.xml

1a40995

Reword messages

f090ebb

Yaron10 mentioned this pull request Oct 22, 2023

Opening multiple files at once will cause previous tabs to disappear #14161

Closed

donho changed the title ~~Make session inaccessible files remembered~~ Make session inaccessible files remembered (part 2/2) Oct 22, 2023

donho added the enhancement Proposed enhancements of existing features label Oct 22, 2023

donho closed this in 0879451 Oct 22, 2023

donho deleted the molsonkiko-fix_session_missing_files_remembering branch October 23, 2023 00:31

donho mentioned this pull request Oct 24, 2023

Selecting several files in Explorer and opening them at once via ENTER key stroke could cause one or more files to be missing and not opened. #14261

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make session inaccessible files remembered (part 2/2) #14252

Make session inaccessible files remembered (part 2/2) #14252

donho commented Oct 20, 2023 •

edited

donho commented Oct 20, 2023

Yaron10 commented Oct 20, 2023 •

edited

donho commented Oct 21, 2023

Yaron10 commented Oct 21, 2023

Yaron10 commented Oct 21, 2023

donho commented Oct 21, 2023

donho commented Oct 21, 2023

Yaron10 commented Oct 21, 2023 •

edited

Yaron10 commented Oct 21, 2023

pryrt commented Oct 21, 2023

Yaron10 commented Oct 21, 2023

donho commented Oct 22, 2023

Yaron10 commented Oct 22, 2023

donho commented Oct 22, 2023 •

edited

donho commented Oct 22, 2023

Yaron10 commented Oct 22, 2023 •

edited

donho commented Oct 23, 2023

Yaron10 commented Oct 23, 2023

Yaron10 commented Oct 23, 2023

donho commented Oct 23, 2023 •

edited

Yaron10 commented Oct 23, 2023 •

edited

Make session inaccessible files remembered (part 2/2) #14252

Make session inaccessible files remembered (part 2/2) #14252

Conversation

donho commented Oct 20, 2023 • edited

donho commented Oct 20, 2023

Yaron10 commented Oct 20, 2023 • edited

donho commented Oct 21, 2023

Yaron10 commented Oct 21, 2023

Yaron10 commented Oct 21, 2023

donho commented Oct 21, 2023

donho commented Oct 21, 2023

Yaron10 commented Oct 21, 2023 • edited

Yaron10 commented Oct 21, 2023

pryrt commented Oct 21, 2023

Yaron10 commented Oct 21, 2023

donho commented Oct 22, 2023

Yaron10 commented Oct 22, 2023

donho commented Oct 22, 2023 • edited

donho commented Oct 22, 2023

Yaron10 commented Oct 22, 2023 • edited

donho commented Oct 23, 2023

Yaron10 commented Oct 23, 2023

Yaron10 commented Oct 23, 2023

donho commented Oct 23, 2023 • edited

Yaron10 commented Oct 23, 2023 • edited

donho commented Oct 20, 2023 •

edited

Yaron10 commented Oct 20, 2023 •

edited

Yaron10 commented Oct 21, 2023 •

edited

donho commented Oct 22, 2023 •

edited

Yaron10 commented Oct 22, 2023 •

edited

donho commented Oct 23, 2023 •

edited

Yaron10 commented Oct 23, 2023 •

edited