Warn before quitting with sessions open #403
Conversation
|
I'd rather have the application just save state and restore upon reopening instead, (and maybe make session restore the toggleable option) |
|
@kjg What if you run commands that is long-running and don't want to accidentally terminate it? Why not have both? 👼 |
|
That's a good point. The way mac OS Terminal.app handles this is by showing a dialog warning about the currently running process ONLY when trying to quit or close the tab in which a process is running. I'd definitely like to see HyperTerm do the same. No preference needed for that, and no warning when there are no processes running. |
|
That's really nice, great UX! I'll check and see what i can do 👼 |
|
I can provide a screen shot or gif of the terminal.app behavior if that will help. Let me know, Thanks! |
|
The problem is how to know if a session is currently running a process, anyone know how to? 👼 |
|
One way of doing so would be to ascertain that the current process title is different from the basename of the shell. If the shell is |
|
There just has to be a better way than checking the title. If there are no hooks into if a process is running in a session, I think the best course of action is to add one and then take advantage of it. |
|
@kjg yeah we got the pid to the process so i'm currently looking into how to use that |
|
@kjg there also has to be a better way of obtaining the title than this hack I wrote: |
|
Ah yeah, there does seem like there should be a better way for getting the title than that haha. I couldn't figure it out quickly though. |
|
I'm looking at https://github.com/Gottox/child_pty to see how hard it would be to make it come straight from there instead. |
timothyis
added
the
👩🔬 Status: In Progress
| app.on('before-quit', (event) => { | ||
| const windows = BrowserWindow.getAllWindows().filter(win => win.sessions); | ||
|
|
||
| if (!quitIsConfirmed && Boolean(windows.length)) { |
There was a problem hiding this comment.
Boolean seems overly-clever here. Prefer windows.length > 0.
|
Closing this now, I will submit an updated version during #hacktoberfest 🎉 |
|
any news on this? |
|
Totally forgot this @jonaswindey 😄 |
Fixes #399, #67
Im putting this up here to discuss if we should put an option in the menu like chrome
Or if we just should just have a config option for it in
hyperterm.jsIf we want to have it so that the checkbox changes your settings, we need to do like hpm to read and change hyperterm.js
However, with that approach there can be 🐎 race conditions, so i think we should wait with that.