run app appargs fix for -- double-dash (#268)

* the usage info for `pipx run`, instead of `app` and `appargs`, now only shows `app ...` with one line of help text.
* Now, if you put a -- directly after app, it will be passed to the app as the first apparg.

src/pipx/commands/commands.py

@@ -46,7 +46,7 @@
 def run(
     app: str,
     package_or_url: str,
-    binary_args: List[str],
+    app_args: List[str],
     python: str,
     pip_args: List[str],
     venv_args: List[str],
@@ -88,7 +88,7 @@ def run(
         logging.info(
             f"Using app in local __pypackages__ directory at {str(pypackage_bin_path)}"
         )
-        return run_pypackage_bin(pypackage_bin_path, binary_args)
+        return run_pypackage_bin(pypackage_bin_path, app_args)
     if pypackages:
         raise PipxError(
             f"'--pypackages' flag was passed, but {str(pypackage_bin_path)!r} was "
@@ -104,14 +104,14 @@ def run(
 
     if bin_path.exists():
         logging.info(f"Reusing cached venv {venv_dir}")
-        retval = venv.run_app(app, binary_args)
+        retval = venv.run_app(app, app_args)
     else:
         logging.info(f"venv location is {venv_dir}")
         retval = _download_and_run(
             Path(venv_dir),
             package_or_url,
             app,
-            binary_args,
+            app_args,
             python,
             pip_args,
             venv_args,
@@ -127,7 +127,7 @@ def _download_and_run(
     venv_dir: Path,
     package_or_url: str,
     app: str,
-    binary_args: List[str],
+    app_args: List[str],
     python: str,
     pip_args: List[str],
     venv_args: List[str],
@@ -159,7 +159,7 @@ def _download_and_run(
             "Available executable scripts: "
             f"{', '.join(b for b in apps)}"
         )
-    return venv.run_app(app, binary_args)
+    return venv.run_app(app, app_args)
 
 
 def _get_temporary_venv_path(

src/pipx/main.py

@@ -8,6 +8,7 @@
 import logging
 from pkg_resources import parse_version
 import shlex
+import re
 import sys
 import textwrap
 import urllib.parse
@@ -139,13 +140,15 @@ def run_pipx_command(args):  # noqa: C901
 
     if args.command == "run":
         package_or_url = (
-            args.spec if ("spec" in args and args.spec is not None) else args.app
+            args.spec
+            if ("spec" in args and args.spec is not None)
+            else args.app_with_args[0]
         )
         use_cache = not args.no_cache
         return commands.run(
-            args.app,
+            args.app_with_args[0],
             package_or_url,
-            args.appargs,
+            args.app_with_args[1:],
             args.python,
             pip_args,
             venv_args,
@@ -434,11 +437,11 @@ def _add_run(subparsers):
         action="store_true",
         help="Do not re-use cached virtual environment if it exists",
     )
-    p.add_argument("app", help="app/package name")
     p.add_argument(
-        "appargs",
+        "app_with_args",
+        metavar="app ...",
         nargs=argparse.REMAINDER,
-        help="arguments passed to the application when it is invoked",
+        help="app/package name and any arguments to be passed to it",
         default=[],
     )
     p.add_argument(
@@ -454,6 +457,12 @@ def _add_run(subparsers):
         help="The Python version to run package's CLI app with. Must be v3.5+.",
     )
     add_pip_venv_args(p)
+    p.set_defaults(subparser=p)
+
+    # modify usage text to show required app argument
+    p.usage = re.sub(r"^usage: ", "", p.format_usage())
+    # add a double-dash to usage text to show requirement before app
+    p.usage = re.sub(r"\.\.\.", "app ...", p.usage)
 
 
 def _add_runpip(subparsers, autocomplete_list_of_installed_packages):
@@ -555,13 +564,28 @@ def setup(args):
         )
 
 
+def check_args(parsed_pipx_args: argparse.Namespace):
+    if parsed_pipx_args.command == "run":
+        # we manually discard a first -- because using nargs=argparse.REMAINDER
+        #   will not do it automatically
+        if parsed_pipx_args.app_with_args and parsed_pipx_args.app_with_args[0] == "--":
+            parsed_pipx_args.app_with_args.pop(0)
+        # since we would like app to be required but not in a separate argparse
+        #   add_argument, we implement our own missing required arg error
+        if not parsed_pipx_args.app_with_args:
+            parsed_pipx_args.subparser.error(
+                "the following arguments are required: app"
+            )
+
+
 def cli() -> int:
     """Entry point from command line"""
     try:
         parser = get_command_parser()
         argcomplete.autocomplete(parser)
         parsed_pipx_args = parser.parse_args()
         setup(parsed_pipx_args)
+        check_args(parsed_pipx_args)
         if not parsed_pipx_args.command:
             parser.print_help()
             return 1

tests/test_run.py

@@ -9,6 +9,8 @@
 
 from helpers import run_pipx_cli
 
+import pipx.main
+
 
 def test_help_text(pipx_temp_env, monkeypatch, capsys):
     mock_exit = mock.Mock(side_effect=ValueError("raised in test to exit early"))
@@ -47,6 +49,34 @@ def test_run_script_from_internet(pipx_temp_env, capsys):
     )
 
 
+@pytest.mark.parametrize(
+    "input_run_args,expected_app_with_args",
+    [
+        (["--", "pycowsay", "--", "hello"], ["pycowsay", "--", "hello"]),
+        (["--", "pycowsay", "--", "--", "hello"], ["pycowsay", "--", "--", "hello"]),
+        (["--", "pycowsay", "hello", "--"], ["pycowsay", "hello", "--"]),
+        (["--", "pycowsay", "hello", "--", "--"], ["pycowsay", "hello", "--", "--"]),
+        (["--", "pycowsay", "--"], ["pycowsay", "--"]),
+        (["--", "pycowsay", "--", "--"], ["pycowsay", "--", "--"]),
+        (["pycowsay", "--", "hello"], ["pycowsay", "--", "hello"]),
+        (["pycowsay", "--", "--", "hello"], ["pycowsay", "--", "--", "hello"]),
+        (["pycowsay", "hello", "--"], ["pycowsay", "hello", "--"]),
+        (["pycowsay", "hello", "--", "--"], ["pycowsay", "hello", "--", "--"]),
+        (["pycowsay", "--"], ["pycowsay", "--"]),
+        (["pycowsay", "--", "--"], ["pycowsay", "--", "--"]),
+        (["--", "--", "pycowsay", "--"], ["--", "pycowsay", "--"]),
+    ],
+)
+def test_appargs_doubledash(
+    pipx_temp_env, capsys, monkeypatch, input_run_args, expected_app_with_args
+):
+    parser = pipx.main.get_command_parser()
+    monkeypatch.setattr(sys, "argv", ["pipx", "run"] + input_run_args)
+    parsed_pipx_args = parser.parse_args()
+    pipx.main.check_args(parsed_pipx_args)
+    assert parsed_pipx_args.app_with_args == expected_app_with_args
+
+
 def test_run_ensure_null_pythonpath():
     env = os.environ.copy()
     env["PYTHONPATH"] = "test"
@@ -72,7 +102,7 @@ def test_run_ensure_null_pythonpath():
 
 # packages listed roughly in order of increasing test duration
 @pytest.mark.parametrize(
-    "package, package_or_url, app_args",
+    "package, package_or_url, app_appargs",
     [
         ("pycowsay", "pycowsay", ["pycowsay", "hello"]),
         ("shell-functools", "shell-functools", ["filter", "--help"]),
@@ -86,11 +116,11 @@ def test_run_ensure_null_pythonpath():
     ],
 )
 def test_package_determination(
-    caplog, pipx_temp_env, package, package_or_url, app_args
+    caplog, pipx_temp_env, package, package_or_url, app_appargs
 ):
     caplog.set_level(logging.INFO)
 
-    run_pipx_cli(["run", "--verbose", "--spec", package_or_url, "--"] + app_args)
+    run_pipx_cli(["run", "--verbose", "--spec", package_or_url, "--"] + app_appargs)
 
     assert "Cannot determine package name" not in caplog.text
     assert f"Determined package name: '{package}'" in caplog.text