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

4 files changed, 135 insertions, 55 deletions

diff --git a/Doc/library/webbrowser.rst b/Doc/library/webbrowser.rst
index c1c4619..3775d9f 100644
--- a/Doc/library/webbrowser.rst
+++ b/Doc/library/webbrowser.rst
@@ -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"
 
diff --git a/Lib/test/test_webbrowser.py b/Lib/test/test_webbrowser.py
index a1bccb5..8496652 100644
--- a/Lib/test/test_webbrowser.py
+++ b/Lib/test/test_webbrowser.py
@@ -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)
@@ -98,6 +100,15 @@ class ChromeCommandTest(CommandTestMixin, unittest.TestCase):
                    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):
 
@@ -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")
@@ -342,7 +353,6 @@ class BrowserRegistrationTest(unittest.TestCase):
     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
@@ -423,5 +433,62 @@ class ImportTest(unittest.TestCase):
             self.assertEqual(webbrowser.get().name, sys.executable)
 
 
-if __name__=='__main__':
+class CliTest(unittest.TestCase):
+    def test_parse_args(self):
+        for command, url, 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),
+        ]:
+            args = webbrowser.parse_args(shlex.split(command))
+
+            self.assertEqual(args.url, url)
+            self.assertEqual(args.new_win, new_win)
+
+    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",
+            # Ensure ambiguous shortening fails
+            "https://example.com --new",
+        ]:
+            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()
diff --git a/Lib/webbrowser.py b/Lib/webbrowser.py
index 7ef80a8..b7fbc41 100755
--- a/Lib/webbrowser.py
+++ b/Lib/webbrowser.py
@@ -11,14 +11,17 @@ import threading
 
 __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:
@@ -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:
@@ -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 *".
@@ -87,6 +92,7 @@ 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.
 
@@ -94,6 +100,7 @@ def open_new(url):
     """
     return open(url, 1)
 
+
 def open_new_tab(url):
     """Open url in a new page ("tab") of the default browser.
 
@@ -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']
@@ -197,7 +204,7 @@ class BackgroundBrowser(GenericBrowser):
             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
 
@@ -225,7 +232,8 @@ class UnixBrowser(BaseBrowser):
             # 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
 
@@ -266,8 +274,8 @@ class UnixBrowser(BaseBrowser):
             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]
@@ -302,7 +310,7 @@ class Epiphany(UnixBrowser):
 
 
 class Chrome(UnixBrowser):
-    "Launcher class for Google Chrome browser."
+    """Launcher class for Google Chrome browser."""
 
     remote_args = ['%action', '%s']
     remote_action = ""
@@ -310,11 +318,12 @@ class Chrome(UnixBrowser):
     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 = ""
@@ -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 = ""
@@ -387,11 +396,11 @@ class Konqueror(BaseBrowser):
         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 = ""
@@ -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"))
 
@@ -514,7 +522,8 @@ 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
@@ -584,15 +593,16 @@ if sys.platform == 'darwin':
 
         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:
@@ -667,33 +677,31 @@ if sys.platform == "ios":
             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)
-    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)
+def parse_args(arg_list: list[str] | None):
+    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_const",
+                       const=1, default=0, dest="new_win",
+                       help="open new window")
+    group.add_argument("-t", "--new-tab", action="store_const",
+                       const=2, default=0, dest="new_win",
+                       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)
+
+    open(args.url, args.new_win)
 
     print("\a")
 
+
 if __name__ == "__main__":
     main()
diff --git a/Misc/NEWS.d/next/Library/2024-03-20-00-11-39.gh-issue-68583.mIlxxb.rst b/Misc/NEWS.d/next/Library/2024-03-20-00-11-39.gh-issue-68583.mIlxxb.rst
new file mode 100644
index 0000000..12caed7
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2024-03-20-00-11-39.gh-issue-68583.mIlxxb.rst
@@ -0,0 +1,2 @@
+webbrowser CLI: replace getopt with argparse, add long options. Patch by
+Hugo van Kemenade.