-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Automatically launch Spyder after installation (Installers) #21084
Conversation
565cfc1
to
a5807c3
Compare
@mrclary, I pushed directly to your branch a commit to improve the message we show on Linux when the installer finishes. This was bothering me a bit the last times I tested the installer and it was not easy to leave a suggestion for it here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrclary, one small comment from me, the rest looks good.
I also forgot to mention that if you have comments or suggestions about the changes I pushed to your branch, please let me know about them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mrclary ! Checked locally on Windows and Spyder launches automatically! 👍
However, if understand correctly. with this Spyder will always open after installation? There is no possibility to have a user confirmation before launch Spyder, right? Also, seems like the installer window is still present after Spyder opens and you have to click next to see the last installer page and close it:
Other than those pointers, which probably are caused do to the way constructor
post install script works on Windows so there is not much we can do here about that, this LGTM 👍
@dalthviz, thanks for reviewing this PR.
Correct. I think this could be fixed for Windows if we used an nsis template. I don't think
That is correct. I may be able to devise a solution for all platforms to delay launching Spyder until the installer is closed. 👀 |
@dalthviz, I just pushed up another commit that may resolve this issue. Another process is started that waits for the graphical installer to exit before starting Spyder. |
I found a bug while testing this (related to PR #20933) and I solve it on PR #21092. Besides that, now Spyder launches after installation (which is great!), but the $ spyder
gtk-launch: missing application name
Try "gtk-launch --help" for more information. I'd prefer to not use
|
I just noticed that. Strange that it doesn't throw that error when bootstrapping...
Ohh, that's right. Will revert.
I'll look at these errors. Thanks for investigating this @ccordoba12 😄 |
I think that's because updates are not run on DEV mode. |
834368c
to
f5e34b8
Compare
Awesome! However, checking again, seems like a cmd shows up in the task bar and you can click on it (I guess is related with the waiting process?). Is there something that could be done to prevent that cmd from showing/being shown in the task bar? Anyhow, I think is nice that now Spyder launches after the installer window finishes/is closed 👍 |
@ccordoba12 @dalthviz, I think all the issues have been worked out now. Additionally, Spyder should delay starting until the graphical installer exits (macOS and Windows). |
That is correct; it is the waiting process. It is not possible to hide the cmd.exe window, only minimize it under these circumstances. However, I could try using a powershell process instead; maybe that will allow the hidden window... 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last, small suggestion from me, the rest is working as expected. Thanks @mrclary!
@dalthviz, so switching to powershell did allow me to eliminate the minimized window. However, there is still a temporary "flash" of the powershell window. This is the same issue @jaimergp and I ran into with the lnk target and |
Checked locally and for sure is an improvement! Even if it shows for a moment, I would say is better since users will not be able to interact with a cmd 👍 |
This prevents Spyder from being killed when/if the terminal shell terminates.
I don't know if /B (don't open cmd window) has an effect in this context, but it doesn't hurt.
* Use SHARED_INSTALLER_TEMP (TMPDIR environment variable is not available to post-install script). * Correct the conditional with pgrep
Use posix compliant lower case parameter substitution. Fix logic error
Note that a powershell console still "flashes" briefly before being hidden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works like a charm for me now, thanks @mrclary!
@dalthviz, are you good to merge now? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mrclary !
Automatically launch Spyder after install