Skip to content
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

gh-68583: webbrowser: replace getopt with argparse, add long options #117047

Merged
merged 17 commits into from
Apr 13, 2024
Merged
9 changes: 6 additions & 3 deletions Doc/library/webbrowser.rst
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ a new tab, with the browser being brought to the foreground. The use of the

The script :program:`webbrowser` can be used as a command-line interface for the
module. It accepts a URL as the argument. It accepts the following optional
parameters: ``-n`` opens the URL in a new browser window, if possible;
``-t`` opens the URL in a new browser page ("tab"). The options are,
naturally, mutually exclusive. Usage example::
parameters:

* ``-n``/``--new-window`` opens the URL in a new browser window, if possible.
* ``-t``/``--new-tab`` opens the URL in a new browser page ("tab").

The options are, naturally, mutually exclusive. Usage example::

python -m webbrowser -t "https://www.python.org"

Expand Down
88 changes: 77 additions & 11 deletions Lib/test/test_webbrowser.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import webbrowser
import unittest
import os
import sys
import re
import shlex
import subprocess
from unittest import mock
import sys
import unittest
import webbrowser
from test import support
from test.support import is_apple_mobile
from test.support import import_helper
from test.support import is_apple_mobile
from test.support import os_helper
from test.support import requires_subprocess
from test.support import threading_helper
from unittest import mock

# The webbrowser module uses threading locks
threading_helper.requires_working_threading(module=True)
Expand Down Expand Up @@ -98,6 +100,15 @@ def test_open_new_tab(self):
options=[],
arguments=[URL])

def test_open_bad_new_parameter(self):
with self.assertRaisesRegex(webbrowser.Error,
re.escape("Bad 'new' parameter to open(); "
"expected 0, 1, or 2, got 999")):
self._test('open',
options=[],
arguments=[URL],
kw=dict(new=999))


class EdgeCommandTest(CommandTestMixin, unittest.TestCase):

Expand Down Expand Up @@ -205,22 +216,22 @@ class ELinksCommandTest(CommandTestMixin, unittest.TestCase):

def test_open(self):
self._test('open', options=['-remote'],
arguments=['openURL({})'.format(URL)])
arguments=[f'openURL({URL})'])

def test_open_with_autoraise_false(self):
self._test('open',
options=['-remote'],
arguments=['openURL({})'.format(URL)])
arguments=[f'openURL({URL})'])

def test_open_new(self):
self._test('open_new',
options=['-remote'],
arguments=['openURL({},new-window)'.format(URL)])
arguments=[f'openURL({URL},new-window)'])

def test_open_new_tab(self):
self._test('open_new_tab',
options=['-remote'],
arguments=['openURL({},new-tab)'.format(URL)])
arguments=[f'openURL({URL},new-tab)'])


@unittest.skipUnless(sys.platform == "ios", "Test only applicable to iOS")
Expand Down Expand Up @@ -342,7 +353,6 @@ def test_register_default(self):
def test_register_preferred(self):
self._check_registration(preferred=True)


@unittest.skipUnless(sys.platform == "darwin", "macOS specific test")
def test_no_xdg_settings_on_macOS(self):
# On macOS webbrowser should not use xdg-settings to
Expand Down Expand Up @@ -423,5 +433,61 @@ def test_environment_preferred(self):
self.assertEqual(webbrowser.get().name, sys.executable)


if __name__=='__main__':
class CliTest(unittest.TestCase):
def test_parse_args(self):
for command, url, new_window, new_tab in [
# No optional arguments
("https://example.com", "https://example.com", False, False),
# Each optional argument
("https://example.com -n", "https://example.com", True, False),
("-n https://example.com", "https://example.com", True, False),
("https://example.com -t", "https://example.com", False, True),
("-t https://example.com", "https://example.com", False, True),
# Long form
("https://example.com --new-window", "https://example.com", True, False),
("--new-window https://example.com", "https://example.com", True, False),
("https://example.com --new-tab", "https://example.com", False, True),
("--new-tab https://example.com", "https://example.com", False, True),
]:
args = webbrowser.parse_args(shlex.split(command))

self.assertEqual(args.url, url)
self.assertEqual(args.new_window, new_window)
self.assertEqual(args.new_tab, new_tab)

def test_parse_args_error(self):
for command in [
# Arguments must not both be given
"https://example.com -n -t",
"https://example.com --new-window --new-tab",
"https://example.com -n --new-tab",
"https://example.com --new-window -t",
]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argparse allows argument shortening and handles ambiguous shortenings as one might hope, but it might be nice to confirm that --new fails properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I didn't know about that feature :) I've added a test case.

with self.assertRaises(SystemExit):
webbrowser.parse_args(shlex.split(command))

def test_main(self):
for command, expected_url, expected_new_win in [
# No optional arguments
("https://example.com", "https://example.com", 0),
# Each optional argument
("https://example.com -n", "https://example.com", 1),
("-n https://example.com", "https://example.com", 1),
("https://example.com -t", "https://example.com", 2),
("-t https://example.com", "https://example.com", 2),
# Long form
("https://example.com --new-window", "https://example.com", 1),
("--new-window https://example.com", "https://example.com", 1),
("https://example.com --new-tab", "https://example.com", 2),
("--new-tab https://example.com", "https://example.com", 2),
]:
with (
mock.patch("webbrowser.open", return_value=None) as mock_open,
mock.patch("builtins.print", return_value=None),
):
webbrowser.main(shlex.split(command))
mock_open.assert_called_once_with(expected_url, expected_new_win)


if __name__ == '__main__':
unittest.main()
91 changes: 51 additions & 40 deletions Lib/webbrowser.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@

__all__ = ["Error", "open", "open_new", "open_new_tab", "get", "register"]


class Error(Exception):
pass


_lock = threading.RLock()
_browsers = {} # Dictionary of available browser controllers
_tryorder = None # Preference order of available browsers
_os_preferred_browser = None # The preferred browser


def register(name, klass, instance=None, *, preferred=False):
"""Register a browser connector."""
with _lock:
Expand All @@ -34,6 +37,7 @@ def register(name, klass, instance=None, *, preferred=False):
else:
_tryorder.append(name)


def get(using=None):
"""Return a browser launcher instance appropriate for the environment."""
if _tryorder is None:
Expand Down Expand Up @@ -64,6 +68,7 @@ def get(using=None):
return command[0]()
raise Error("could not locate runnable browser")


# Please note: the following definition hides a builtin function.
# It is recommended one does "import webbrowser" and uses webbrowser.open(url)
# instead of "from webbrowser import *".
Expand All @@ -87,13 +92,15 @@ def open(url, new=0, autoraise=True):
return True
return False


def open_new(url):
"""Open url in a new window of the default browser.

If not possible, then open url in the only browser window.
"""
return open(url, 1)


def open_new_tab(url):
"""Open url in a new page ("tab") of the default browser.

Expand Down Expand Up @@ -136,7 +143,7 @@ def _synthesize(browser, *, preferred=False):

# General parent classes

class BaseBrowser(object):
class BaseBrowser:
"""Parent class for all browsers. Do not use directly."""

args = ['%s']
Expand Down Expand Up @@ -197,7 +204,7 @@ def open(self, url, new=0, autoraise=True):
else:
p = subprocess.Popen(cmdline, close_fds=True,
start_new_session=True)
return (p.poll() is None)
return p.poll() is None
except OSError:
return False

Expand Down Expand Up @@ -225,7 +232,8 @@ def _invoke(self, args, remote, autoraise, url=None):
# use autoraise argument only for remote invocation
autoraise = int(autoraise)
opt = self.raise_opts[autoraise]
if opt: raise_opt = [opt]
if opt:
raise_opt = [opt]

cmdline = [self.name] + raise_opt + args

Expand Down Expand Up @@ -266,8 +274,8 @@ def open(self, url, new=0, autoraise=True):
else:
action = self.remote_action_newtab
else:
raise Error("Bad 'new' parameter to open(); " +
"expected 0, 1, or 2, got %s" % new)
raise Error("Bad 'new' parameter to open(); "
f"expected 0, 1, or 2, got {new}")

args = [arg.replace("%s", url).replace("%action", action)
for arg in self.remote_args]
Expand Down Expand Up @@ -302,19 +310,20 @@ class Epiphany(UnixBrowser):


class Chrome(UnixBrowser):
"Launcher class for Google Chrome browser."
"""Launcher class for Google Chrome browser."""

remote_args = ['%action', '%s']
remote_action = ""
remote_action_newwin = "--new-window"
remote_action_newtab = ""
background = True


Chromium = Chrome


class Opera(UnixBrowser):
"Launcher class for Opera browser."
"""Launcher class for Opera browser."""

remote_args = ['%action', '%s']
remote_action = ""
Expand All @@ -324,7 +333,7 @@ class Opera(UnixBrowser):


class Elinks(UnixBrowser):
"Launcher class for Elinks browsers."
"""Launcher class for Elinks browsers."""

remote_args = ['-remote', 'openURL(%s%action)']
remote_action = ""
Expand Down Expand Up @@ -387,11 +396,11 @@ def open(self, url, new=0, autoraise=True):
except OSError:
return False
else:
return (p.poll() is None)
return p.poll() is None


class Edge(UnixBrowser):
"Launcher class for Microsoft Edge browser."
"""Launcher class for Microsoft Edge browser."""

remote_args = ['%action', '%s']
remote_action = ""
Expand Down Expand Up @@ -461,7 +470,6 @@ def register_X_browsers():
if shutil.which("opera"):
register("opera", None, Opera("opera"))


if shutil.which("microsoft-edge"):
register("microsoft-edge", None, Edge("microsoft-edge"))

Expand Down Expand Up @@ -514,7 +522,7 @@ def register_standard_browsers():
cmd = "xdg-settings get default-web-browser".split()
raw_result = subprocess.check_output(cmd, stderr=subprocess.DEVNULL)
result = raw_result.decode().strip()
except (FileNotFoundError, subprocess.CalledProcessError, PermissionError, NotADirectoryError) :
except (FileNotFoundError, subprocess.CalledProcessError, PermissionError, NotADirectoryError):
pass
else:
global _os_preferred_browser
Expand Down Expand Up @@ -584,15 +592,16 @@ def __init__(self, name='default'):

def open(self, url, new=0, autoraise=True):
sys.audit("webbrowser.open", url)
url = url.replace('"', '%22')
if self.name == 'default':
script = 'open location "%s"' % url.replace('"', '%22') # opens in default browser
script = f'open location "{url}"' # opens in default browser
else:
script = f'''
tell application "%s"
tell application "{self.name}"
activate
open location "%s"
open location "{url}"
end
'''%(self.name, url.replace('"', '%22'))
'''

osapipe = os.popen("osascript", "w")
if osapipe is None:
Expand Down Expand Up @@ -667,33 +676,35 @@ def open(self, url, new=0, autoraise=True):
return True


def main():
import getopt
usage = """Usage: %s [-n | -t | -h] url
-n: open new window
-t: open new tab
-h, --help: show help""" % sys.argv[0]
try:
opts, args = getopt.getopt(sys.argv[1:], 'ntdh',['help'])
except getopt.error as msg:
print(msg, file=sys.stderr)
print(usage, file=sys.stderr)
sys.exit(1)
def parse_args(arg_list: list[str] | None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's our current policy on stdlib type hints?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python/devguide#1304 says in general don't add, but there are some specific exceptions, but that the policy is undocumented. I think these might be okay because they're "simple" and internal, but I'm also happy to remove if you'd prefer?

import argparse
parser = argparse.ArgumentParser(description="Open URL in a web browser.")
parser.add_argument("url", help="URL to open")

group = parser.add_mutually_exclusive_group()
group.add_argument("-n", "--new-window", action="store_true",
help="open new window")
group.add_argument("-t", "--new-tab", action="store_true",
help="open new tab")

args = parser.parse_args(arg_list)

return args


def main(arg_list: list[str] | None = None):
args = parse_args(arg_list)

new_win = 0
for o, a in opts:
if o == '-n': new_win = 1
elif o == '-t': new_win = 2
elif o == '-h' or o == '--help':
print(usage, file=sys.stderr)
sys.exit()
if len(args) != 1:
print(usage, file=sys.stderr)
sys.exit(1)

url = args[0]
open(url, new_win)
if args.new_window:
new_win = 1
elif args.new_tab:
new_win = 2

open(args.url, new_win)

print("\a")


if __name__ == "__main__":
main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
webbrowser CLI: replace getopt with argparse, add long options. Patch by
Hugo van Kemenade.
Loading