bpo-24241: Preferred X browser#85
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:
Thanks again to your contribution and we look forward to looking at it! |
Sorry, something went wrong.
|
I've tied my bpo 'daves' account to github 'davesteele'. The contributor form was received 2015-07-17.00:00:00. |
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #85 +/- ##
==========================================
- Coverage 82.38% 82.38% -0.01%
==========================================
Files 1428 1428
Lines 351138 351143 +5
==========================================
- Hits 289291 289275 -16
- Misses 61847 61868 +21Continue to review full report at Codecov.
|
Sorry, something went wrong.
ultimatecoder
left a comment
There was a problem hiding this comment.
Found improvements to the code.
Sorry, something went wrong.
ultimatecoder
left a comment
There was a problem hiding this comment.
I will suggest updating the documentation for method register at Doc/library/webbrowser.rst
Sorry, something went wrong.
|
I've added documentation for the register() priority argument. |
Sorry, something went wrong.
ncoghlan
left a comment
There was a problem hiding this comment.
I like the general idea of this patch, but think it would be more self-explanatory with some changes to the specific implementation details.
Sorry, something went wrong.
Codecov Report
@@ Coverage Diff @@
## master #85 +/- ##
==========================================
- Coverage 82.38% 82.37% -0.01%
==========================================
Files 1428 1428
Lines 351138 351206 +68
==========================================
+ Hits 289291 289317 +26
- Misses 61847 61889 +42Continue to review full report at Codecov.
|
Sorry, something went wrong.
|
@ncoghlan, I've made some changes based on your review, and pushed back on a few others. You be the final arbiter on the issues. Let me know. |
Sorry, something went wrong.
|
+1 on leaving the helper variable as a single string - I've suggested a name change and an additional explanatory comment to cover some of the subtleties for future maintainers. For the The commit message also still needs adjustment to better cover the changes being made (i.e. both the If you're so inclined, you could also add an entry to the 3.7 What's New for this change (otherwise I'll just add it along with the NEWS entry when pushing the change) |
Sorry, something went wrong.
Replace the existing undocumented tri-state 'try_order' parameter with the boolean keyword-only 'preferred' parameter. Setting it to True places the browser at the front of the list, preferring it as the return to a subsequent get() call. 'preferred' is added to the documentation.
The traditional first entry in the tryorder queue is xdg-open, which doesn't support new window. Make the default browser first in the try list, to properly support these options.
795ee09 to
871455f
Compare
February 24, 2017 21:37
|
@ncoghlan, I've reworked, reworded, and rebased the commits based on your suggestions. |
Sorry, something went wrong.
|
The codecov complaints are due to CI not running on Mac OS X, so this looks good to me now - merging! |
Sorry, something went wrong.
|
Thanks for your help on this. |
Sorry, something went wrong.
|
Added the sprint label, as this PR was submitted at the PyCon Pune 2017 core development sprint. |
Sorry, something went wrong.
…WatchdogEx Fix issue python#85: don't schedule already scheduled tasklets. Ensure that the internal watchdog list is empty before and after all test cases of class TestNewWatchdog. Fix test case TestNewWatchdog.test_watchdog_priority_{hard|soft}. The timeout value was much to short to build up a list of watchdogs. The new test cases TestNewWatchdog.test_schedule_deeper_{hard|soft} triggers the assertion failure reliably. They are skipped for now. https://bitbucket.org/stackless-dev/stackless/issues/85 (grafted from 2235702eb90cb0709dd40544b3952a956f2032a9 and 3fb55ce5b2d4)
Sometimes we need to acquire locks before the current PyThreadState is set. This moves the waiter data to it's own struct. It's shared between PyThreadStates on the same thread, such as in multi-interpreter settings. Fixes #85
Add github workflow to build and publish docker image
When calling webbrowser.open*(), the module goes through a list of installed browsers, and uses the first one that succeeds, to process the request.
The first 'browsers' in the 'X' list are 'xdg-open' and others of that ilk. The problem is that they only have one 'open' behavior - the 'new' parameter is ignored ('same window', 'new window', 'new tab').
These commits move the default web browser to the front of the tryorder list, allowing get() behavior to match the documentation.
This same problem was recently fixed in Windows via Issue #8232