-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Automatically open browser when Invoke task starts web server #2764
Conversation
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.
I like the idea of opening the target URL in the user's default browser automatically, but in practice it is not clear to me how to do so while providing a good user experience. For example, in my testing, Python's webbrowser
tried to open Chrome (which does not exist on my workstation) and then opened Firefox (which is not the default browser on my macOS workstation):
0:28: execution error: An error of type -10814 has occurred. (-10814)
69:77: execution error: Can’t get application "chrome". (-1728)
So unless there is a reliable way for Python to determine the user's default browser correctly across macOS, Windows, and Linux operating systems, unfortunately I don't think we will be able to achieve the intended objective.
04c7404
to
1a9c86c
Compare
Hi!
I think the problem you describe is not related with the contribution itself, but with the This PR is bringing more benefits than problems in my opinion, you happen to be unlucky in your setup. It works for me, and I guess it will work for a lot of other people too. If you do not manage to make the Thanks for the review, both comments have been addressed in the updated version. |
I understand that the problem is related to how
I am aware of that workaround. I did not use it because (1) I have never had to do so previously, and (2) that would only solve it for me and not for other Pelican users. Ergo, I don’t consider that to be a viable solution. Providing a semi-broken feature to end users and then telling them that they must set
As I said before, I like the intention and am open to an implementation that works well. In its current form, this implementation does not deliver a good user experience for a significant proportion of users.
Unlucky? That is an… interesting perspective. If the same thing happens to all macOS users, then it’s not really a question of luck, is it? (That is a rhetorical question.) And what about folks on Windows? Are we casting them aside as well? I understand the precarious temptation to hand-wave and dismiss with, “It works on my machine, so there must be a problem with your set-up.“ However, I suggest resisting that temptation. For starters, such assertions are usually wrong. More importantly, it’s easy to say that “It works for me” when you don’t have to respond to a swarm of new issues from folks when it doesn’t work well for them. I, on the other hand, do have to deal with those support requests, so I don’t have the luxury of getting by with “works for me”. “It works on my machine Now that we’ve adjusted our perspective, let’s try to find a solution that also works well on other folks’ workstations, shall we? Since Python’s c.run("open http://{host}:{port}".format(**CONFIG)) For Windows, perhaps the following will work: c.run('explorer "http://{host}:{port}"'.format(**CONFIG)) For Linux, maybe xdg-open is appropriate: c.run('xdg-open "http://{host}:{port}"'.format(**CONFIG)) While that would require a few extra lines of code in
|
Hello Justin, thanks for your answer and your time. I agree that the it works on my machine is an incorrect approach as it can quickly become a maintenance hell as more users report issues. This is why we should spend more time to make sure that the implementation of this proposal works for everyone. However, I also think that one should not reinvent the wheel but use the tools at disposal when possible, and enhance them if they do not behave correctly. The webbrowser module being part of the stdlib, I think we should really make the effort of using it instead of creating a poor clone that will have gone through less more eyes and testing. As the module is 713 lines long, I doubt we could be any smarter with a few lines inside of the I downloaded the Python 3.8.3 source code and had a look at the Inside the OSX specific part, the "open" command you suggested seems to be used if the browser is set to "default". In fact, the command is "open location" as seen in this example: script = 'open location "%s"' % url.replace('"', '%22') # opens in default browser The register order for OSX directly specifies that the 'default' web browser should be the first one to be tested before Chrome, so the above call should occur on your computer before the Chrome call: def register_standard_browsers():
global _tryorder
_tryorder = []
if sys.platform == 'darwin':
register("MacOSX", None, MacOSXOSAScript('default'))
register("chrome", None, MacOSXOSAScript('chrome'))
register("firefox", None, MacOSXOSAScript('firefox'))
register("safari", None, MacOSXOSAScript('safari'))
# OS X can use below Unix support (but we prefer using the OS X
# specific stuff) Maybe the version of Python you are using is not containing the same implementation as this 3.8.3 release. From my understanding of the webbrowser source code, the Any chance "open location" does not work while "open" does work on your OSX system? Please find attached webbrowser.py.txt for your convenience, renamed to bypass Github .py exclusion rule. |
For what it's worth, It does however need the protocol ( So at least on this one, Windows looks like it's the simple solution! |
@MinchinWeb: Many thanks for testing on Windows. Much appreciated. @MicroJoe: My apologies for not coming back to this endeavor sooner. I tested this again just now and found that, for whatever reason, it now functions as intended and opens my default browser. Why that was not the case before… remains a mystery. I think there are just a few changes I'd like to see before merging this PR:
What do you think? |
When the livereload target is invoked, a web brower will be automatically opened pointing to the locally-served website. In case of no web browser can be found by the module, the open call returns False but no exception is raised. This means that it is still possible to call livereload on a remote machine and access it without any error being triggered. Signed-off-by: Romain Porte <microjoe@microjoe.org>
1a9c86c
to
35bbda4
Compare
@justinmayer thanks for the feedback, commit updated with taking into account 1. 2. and 3. remarks. |
@MicroJoe: Thank you for responding so quickly. In the interest of behavior consistency, do you think it makes sense to add this same stanza to the if OPEN_BROWSER_ON_SERVE:
# Open site in default browser
import webbrowser
webbrowser.open("http://{host}:{port}".format(**CONFIG)) |
@justinmayer great idea, added a new commit on top to do that as well. |
Nice work, Romain. Many thanks for your contribution, patience, and willingness to work together to come up with a robust solution. I like your philosophy about fixing things upstream when the opportunity presents itself. It's been a pleasure. ✨ |
When the livereload target is invoked, a web brower will be automatically opened pointing to the locally-served website.
In case of no web browser can be found by the module, the open call returns False but no exception is raised. This means that it is still possible to call livereload on a remote machine and access it without any error being triggered.
Pull Request Checklist
Other fields in the checklist have been removed, as they are considered not applicable for this contribution.