Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Request] Close without saving modified files #81

Closed
digg33 opened this issue Apr 9, 2022 · 40 comments · Fixed by #264
Closed

[Feature Request] Close without saving modified files #81

digg33 opened this issue Apr 9, 2022 · 40 comments · Fixed by #264

Comments

@digg33
Copy link

digg33 commented Apr 9, 2022

The default behaviour in Notepad++ has always been to retain any modified buffers when closed without prompting the user to save. This would be a useful feature to have in order to use NotepadNext as a scratchpad of sorts.

Thanks,
digg33

@dail8859
Copy link
Owner

Thanks for opening this issue.

I know this is a very useful feature in Notepad++ and alot of people like it. I think for the general audience that this fills a huge need to keep temporary notes, have a scratchpad, etc (personally I hate this feature, but that's beside the point 😄). There's a bit of work that will need done beforehand to make this feasible to implement, such as better preference handling and session saving/loading. Once those are working properly then keeping unsaved files would be much easier to implement.

@mikelupe
Copy link

This feature is basically one of the main reasons for me to use Notepad++ (through Wine). I thought this was quite "simply" solved - each unsaved tab exists as a temporary file. Or isn't it that "simple"?

@dail8859
Copy link
Owner

There is some extra 'meta' data that needs associated with the unsaved files. Along with storing the actual text, the Notepad++ session stores things such as:

  • firstVisibleLine
  • xOffset
  • scrollWidth
  • startPos
  • endPos
  • selMode
  • offset
  • wrapCount
  • lang
  • encoding
  • userReadOnly
  • filename
  • backupFilePath
  • originalFileLastModifTimestamp
  • originalFileLastModifTimestampHigh

I don't think all of these would be required immediately, but having session saving/loading capabilities would be needed to make keeping unsaved files much easier. All possible in theory, just some additional groundwork that would help making this functionality possible.

@mikelupe
Copy link

Yes, I see - thanks for the wrap-up :)

@Rcomian
Copy link

Rcomian commented May 3, 2022

Absolutely this feature. As has been mentioned, it's the reason for using/wanting a notepad++ like editor.

@dzek69
Copy link

dzek69 commented Oct 9, 2022

+2 from me and my gf, this is the main reason I even use notepad++ and the main reason I will use this app when I will switch to linux again

even the simplest implementation would be perfect

And something for this post not being just another "me too":
I see the project got to trending list again, maybe it'd be a good idea to open some donations? (not to mention feature request-based donations if you like others to set up your priorities, but it's 100% up to you!)

@bluebat1
Copy link

There is some extra 'meta' data that needs associated with the unsaved files. Along with storing the actual text, the Notepad++ session stores things such as:

  • firstVisibleLine
  • xOffset
  • scrollWidth
  • startPos
  • endPos
  • selMode
  • offset
  • wrapCount
  • lang
  • encoding
  • userReadOnly
  • filename
  • backupFilePath
  • originalFileLastModifTimestamp
  • originalFileLastModifTimestampHigh

I don't think all of these would be required immediately, but having session saving/loading capabilities would be needed to make keeping unsaved files much easier. All possible in theory, just some additional groundwork that would help making this functionality possible.

Would you consider using SQLite for this session stores functionality?

@dail8859
Copy link
Owner

Would you consider using SQLite for this session stores functionality?

SQLite would be too much for the needs. I don't think the root of the problem due to how much data is stored or searching through it.

The built-in QSettings functionality that uses INI files would most likely be sufficient and would be the easiest way to implement it. The bulk of the work would just be implementing the functionality within the application to appropriately store and reload the session state.

Personally, I don't use this feature in Notepad++ so I'm not overly familiar with some of the possible corner cases that would need handled. If I have some free time in the near future, maybe I'll take a quick attempt at the functionality to see what kind of questions/issues I run into.

@dail8859
Copy link
Owner

All,
I've started testing out some changes in #264 to work towards this feature request. This is a first attempt at saving/loading a user's session (does not currently save unsaved files yet) between application restarts. It does reopen previously opened/saved files.

If you are interested in testing these changes (as I am sure there are bugs), use the links at the bottom to download a test build for your system. (If someone wants a flatpak beta release let me know). Please post on #264 if you find any issues or have questions. I intentionally did not try to match all of Notepad++'s behavior identically. I'd rather start simple, and then address concrete use-cases as they arise.

Please note:

  1. This feature is not enabled by default. This needs enabled through the preferences dialog.
  2. This feature does not currently save temporary files (that will come later). This base functionality is needed before temporary files can be properly saved/loaded.

Download

All of these are zip files. You will have to unzip them first.

@dzek69
Copy link

dzek69 commented Nov 10, 2022

After quick testing I couldn't find any bug, I just found something that's probably unfinished:

  • it remembers scroll position, but it doesn't remember cursor position
  • it doesn't remember which tab was left in front (always focuses the last one)

Good work!

Edit: tested on Windows

@dail8859
Copy link
Owner

New download:

Cursor position is restored, and previously active tab is focused.

Again these are all zip files so you'll have to unzip them first.

@dzek69
Copy link

dzek69 commented Nov 14, 2022

Windows and Linux version seems to work flawlessly (I spent just few minutes on them). Thanks!

After you will add a way to remember files without saving them I'd gladly donate to this project and start using it on a daily basis 👍🏻

@CodeCubeNeo
Copy link

Any updates on this? Where can I try this feature. In the settings there is no option to enable it?

@dail8859
Copy link
Owner

The PR builds linked has some basic functionality. It is not in the main release yet. Progress is slow as I've been quite busy lately. The basic 'loading a temp' file functionality is partially working, but there are some non-standard things that just aren't built into Scintilla (the main text editing part of the app) and trying to dig through the Notepad++ source code is quite painful.

@dail8859
Copy link
Owner

dail8859 commented Dec 28, 2022

After rewriting the logic probably 4 times, finding bugs I thought I introduced, and several iterations of code cleanup, I think this is pretty close to "functional".

There are bugs. This should be considered very experimental and could possibly cause you to lose data if you enable the options.

Please test this out and when you run into issues, the better you can describe the problem and the steps to reproduce it, the quicker I can fix it.

Any and all feedback is welcome.

@dail8859
Copy link
Owner

Latest Downloads:

Thanks to @mikelupe for pointing out a couple issues that should be fixed now:

@dail8859 dail8859 linked a pull request Dec 31, 2022 that will close this issue
3 tasks
@dzek69
Copy link

dzek69 commented Jan 4, 2023

@dail8859
I'm a bit late to the party as I was in the middle of setting up my fresh Linux installation, but just wanted to give feedback anyway that this seems stable for me (the version you posted 5 days ago). I'm using Notepad Next as main notepad app for few days and it should stay like that, so if you need to beta test something - please mention me with @dzek69 and I'll be happy to test.

@ekasprzak
Copy link

Yup, this seems to be working great, that's the only function that was keeping me from using Notepad Next as my daily driver, thanks a lot!

Any ETA when this might hit Flatpak build (AppImages don't play nice with gnome dock favorite and for me, this is crucial for my notepad to be as quickly accessible as possible)?

@dzek69
Copy link

dzek69 commented Jan 7, 2023

I wanted to report I had a data loss yestarday, I had just one file open with unsaved data, I restarted my OS and next time I started NN there was nothing.
I did not try to reproduce yet, now I'm a bit too busy.

I wanted to test first before leaving a comment but if somebody (looking at you @ekasprzak) want to rely on this - don't do it yet :)

@dail8859
Copy link
Owner

dail8859 commented Jan 8, 2023

@dzek69 That's unfortunate to hear but I guess I'm not super surprised since this is a brand new feature. Any details would be appreciated if you do happen to track it down some more.

@dail8859
Copy link
Owner

Given there still seems to be some bugs to work out, I'll push a change soon that will at least warn the user of possible data loss when enabling this feature. Data loss is by far the worst possible kind of bug! This way I can still proceed with a new release for users that are wanting to give it a try.

@dzek69
Copy link

dzek69 commented Jan 13, 2023

@dail8859: hey, I discovered how to reproduce it. Well. I even wrote here about that and I was waiting for response, but it looks like I did not press the submit button or GitHub lost it between the bits.

Anyway, if you kill the app with SIGKILL signal it will always loose all the state. That's the tl;dr info.

Example how do I reproduce it:

  1. Open NN
  2. In the terminal do: top -b -n 1 | grep wrap and remember the pid (first number)
  3. In the terminal do: kill -9 INPUT_PID_HERE
  4. You should be able to replace step 2 & 3 with killall -9 AppRun.wrapped, but it seems that process name of NN is kind of generic and that could kill some other apps as well if you happen to run one with the same process name.

BTW: Can you change the process name to something more meaningful?

Bonus thoughts:

  • Notepad++ keeps the never-saved-yet (all the New 1 New 2 etc) files in a backup directory in AppData.
  • I think this is nice idea - it's bulletproof, because it's always on disk, you can nuke the app and it will persist.
  • Bonus points for easy recover if your whole OS is dead
  • More bonus points for naming pattern - like new 6@2022-12-19_153239 - file creation date is in the name, and last modified attribute can be read from filesystem attributes - this once helped me a lot when I thought I lost something important
  • I have no idea how they save the modified state of existing file without actually overriding them - there must be some session data stored as well, that keeps track of that + the order of tabs (you can't figure that out from just filenames)

But I hope there is no need to recreate things the same way and you can solve killing app issue (I guess most Linux OSes will always kill the app when shutting down, so this is something that will happen quite often to users) before releasing the app with this feature.

@mikelupe
Copy link

@dzek69 , @dail8859 I was about to report a similar thing, so I can confirm this.

I realized this happens when shutting down the machine, which basically means notepad-next is recieving a SIGKILL signal.

@dail8859
Copy link
Owner

@dzek69 @mikelupe Thanks for the info!

I'll have to do some research to see what kind of signals are generated in the application itself as the QCoreApplication::aboutToQuit() doesn't seem to trigger under non-graceful circumstances as you've found.

Can you change the process name to something more meaningful?

No clue how to do it or if it is possible but I agree it is very generic and not helpful.

Notepad++ keeps the never-saved-yet (all the New 1 New 2 etc) files in a backup directory in AppData.

On Windows it is very similar though it is called session as I personally think this is different than a 'backup' type of feature (but that's kind of beside the point). According to #264 (comment) on Linux it is saved to ~/.local/share/NotepadNext/NotepadNext/session/

More bonus points for naming pattern - like new 6@2022-12-19_153239 - file creation date is in the name, and last modified attribute can be read from filesystem attributes - this once helped me a lot when I thought I lost something important.

Then UUID was a quick and guaranteed way to create a unique file name. In theory using the name + file create date is mostly safe but still possible to have clashes and loss of data.

before releasing the app with this feature.

v0.6 is now available with this feature. If I can get some of the bugs fixed soon I'm fine pushing out a minor release especially if it is to prevent possible data loss.

@dzek69
Copy link

dzek69 commented Jan 13, 2023

AFAIK the answer to

what kind of signals are generated in the application itself

is "none". Random source: https://stackoverflow.com/questions/15766036/sigkill-signal-handling

@dzek69
Copy link

dzek69 commented Jan 13, 2023

According to #264 (comment) on Linux it is saved to ~/.local/share/NotepadNext/NotepadNext/session/

Right. It's there. And it's updated only on proper shutdown, so if I close the app with some data (like "aaa") and reopen it, change the contents (to "bbb"), kill it and open it again - i will have something - it will just be outdated ("aaa").

I guess you can just save it periodically :)

Last comment in this issue. I didn't want to copy the whole convo to the new one. Sorry for the chaos :)

@mikelupe
Copy link

mikelupe commented Jan 13, 2023

I was thinking about the periodic saving, but this isn't really realiable I guess :) The only reliable way imho would be to save on every key-stroke, but I'm not really sure this is acceptable performance-wise. Or is it?

edit: ev. sigkill handling IS possible in some way? Unfortunately I'm not a developer, but I almost can't imagine this not being an option

@dail8859
Copy link
Owner

The only reliable way imho would be to save on every key-stroke, but I'm not really sure this is acceptable performance-wise.

Definitely not a good idea. :)

sigkill handling IS possible in some way?

Possible to do with some care.

@mikelupe
Copy link

"Definitely not a good idea" - dito :)

Thanks for your work

@dail8859
Copy link
Owner

@dzek69

is "none". Random source: https://stackoverflow.com/questions/15766036/sigkill-signal-handling

Agreed. There are some signals that are a nuclear option and data loss and/or non-standard behavior are expected.

https://man7.org/linux/man-pages/man7/signal.7.html states:

The signals SIGKILL and SIGSTOP cannot be caught, blocked, or ignored.

That being said a graceful system shutdown should send other signals that can be handled. Qt also has Session Management that may be the solution to handling these signals in the recommended Qt way.

@Rcomian
Copy link

Rcomian commented Jan 13, 2023 via email

@dail8859
Copy link
Owner

@Rcomian It is in the Preference dialog. Shouldn't be too hard to find since there isn't much there yet.

that said, when are we saving the modified files?

On application shutdown

@Rcomian
Copy link

Rcomian commented Jan 13, 2023

ah yes, that's gonna leave us vulnerable, unless as i say, we save as we go along.

@mikelupe
Copy link

dail8859 isn't too eager about this either, as "save as we go along" would basically mean "on every key stroke"

"The only reliable way imho would be to save on every key-stroke, but I'm not really sure this is acceptable performance-wise."

"Definitely not a good idea. :)"

@dail8859
Copy link
Owner

@Rcomian

ah yes, that's gonna leave us vulnerable, unless as i say, we save as we go along.

It is meant to save temporary changes between application restarts. If the text you are modifying is important, save it to disk.

@Rcomian
Copy link

Rcomian commented Jan 13, 2023 via email

@dzek69
Copy link

dzek69 commented Jan 13, 2023

I'd go with saving session 3 secs after last key press.

It's a balance between performance degradation caused by instant saves and reliability (if I lost something I just typed it's not that bad, at least I remember it and I should be able to notice if app/power/whatever crashes)

@dzek69
Copy link

dzek69 commented Jan 13, 2023

Trying to handle "sigkill"s AFAIK means "having a wrapper process" that monitors those kills, but for something like that a lot of logic would have to be moved there and still on shutdown OS will kill anyway. It's a risky path that probably leads to nowhere anyway

@dail8859
Copy link
Owner

we're not after nuclear grade data retention

I'm not saying I'm against this, just rather the "saving session state" versus the "back up my files in case of a system/application failure" are slightly different features. Yes there is quite a bit of overlap for sure (maybe even more than I realize?)

@Rcomian

that way, if you press a key, within a few seconds it'll be safe in the
temp area. if you're constantly typing, it'll be saved as you go. no need
for every individual change to be saved.

@dzek69

It's a balance between performance degradation caused by instant saves and reliability (if I lost something I just typed it's not that bad, at least I remember it and I should be able to notice if app/power/whatever crashes)

Agreed and as @Rcomian stated just a reasonable expectation that it'll generally be there is definitely correct.

If someone wanted to open an issue or discussion around more of the backup functionality it is definitely worth discussing.

@dzek69
Copy link

dzek69 commented Jan 13, 2023

I don't expect a backup solution too. In my 3-4 years of using N++ it lost my session once, but my files could still be found inside the backup folder. I think that's ok.

If that would be a session reset every 2 months then i'd prefer not to have this feature at all, "no safety measures" sounds better than "safety measures that sometimes fail", because I'd just know I should save all the time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants