-
Notifications
You must be signed in to change notification settings - Fork 450
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
Fork off webbrowser call in case it fails to return #3537
base: master
Are you sure you want to change the base?
Fork off webbrowser call in case it fails to return #3537
Conversation
This fixes the issue discussed in #3536 (command line browsers hang). As discussed there, there are better things to do than this (open a GUI browser, set the BROWSER variable), but this at least means nikola does not appear to hang. |
Here’s a better idea of a fix: browser = webbrowser.get()
if browser.background or user_on_windows:
browser.open(url)
else:
# Tell user that their browser is mediocre and offer 3 options:
# Standard open (with logs)
# Open without logs (requires forking)
# Don't open, just serve |
I did forget Thanks, I'm going to go and think about this, annoyingly one needs to pick a bit through the hierarchy (I seem to actually have a GenericBrowser, which doesn't have browser.background) -- I'm not sure if GenericBrowser will, in some cases, return promptly, in which case this might be a lost cause (without an upstream fix of some kind). |
You could do this:
|
Hi, Unfortunately, it looks like command line browsers, and browsers defined with the BROWSER variable, are both handled as The best option I can think of is to specifically check |
|
11922fa
to
018a05c
Compare
Argh! :) I hoped all linux distros would install that, maybe it's a debian thing? i could just hardwire ["www-browser", "links", "elinks", "lynx", "w3m"], based on https://github.com/python/cpython/blob/3.9/Lib/webbrowser.py ? Sorry, this is clearly taking longer than it's probably worth, but I feel invested now. |
Hardcoding those browsers sounds like a good idea to me. (And feeling invested in a PR, big or small, is certainly great for the project!) |
If the browser is a command line browser, opening it will pause Python which stops Nikola serving pages.
018a05c
to
562eb96
Compare
And then running WSL seems to ship with https://github.com/wslutilities/wslu by default, and it does the right thing, but installing eg. |
OK, so if your www-browser is firefox, and mine is lynx, as you say this isn't fixable nicely by us. I also previous found a page that said www-browser should be command line, but in debian it can be provided by chromium and firefox, so that's off as well. Hmm.. that brings us back to (I think) -- do the "open with fork", have the logs mixed in if someone has a terminal, and assume they'll realise that means they have to go and run the browser somewhere else if they don't want messages mixed in. While that's bad, it's an improvement on the current (in my opinion), where it seems to hang so it's not obvious what's to blame. |
I think we could include |
open ? (x-www-browser ?)This is from a Debian point of view. My machine is running the current stable Debian version Bullseye. I suggest calling
If a browser is to be called directly - well, my machine does not have
What does exist in my system is
so you may consider adding that to a default list. Unrelated, won't help us:
|
Pull Request Checklist
Description
calls to webbrowser.open do not return for command-line browser, so run it in a fork