Reimplement deprecated subcommands, properly, in `click` (10c7604) - derivepassphrase.git

Reimplement deprecated subcommands, properly, in `click`

Marco Ricci commited on 2024-12-07 08:50:03
Zeige 2 geänderte Dateien mit 146 Einfügungen und 37 Löschungen.

Our previous method of dispatching to subcommands was not tying the
supercommand and the subcommand together properly; each command
essentially ran in its own isolated context.  This lead to unnecessary
double-parsing of the command-line arguments, and to `click` becoming
confused as to what the full command-line actually was; in particular,
the help and usage messages gave a wrong command summary.

Instead, we now use custom `click.Group` objects as the supercommands
which default to the "vault" subcommand (with a deprecation warning) if
no explicit subcommand could be matched, rather than throwing an error;
this all happens internally using the normal mechanism for subcommand
dispatch.  Additionally, if the supercommand callback actually is
invoked -- which only happens if the supercommand is given without any
arguments -- we emit a deprecation warning and dispatch manually to the
"vault" subcommand using the same code path as the `click`
implementation would otherwise do.  We assert that the old and new code
paths work via new tests as necessary, particularly for the
"supercommand without arguments" case.

This implementation necessarily messes with some `click` internals,
duplicating the code as it currently stands.  Being a non-public API,
there is of course no guarantee that this will work with future versions
of `click`.  However, we use this solely to implement a compatibility
interface, which should vanish with `derivepassphrase` v1.0 anyway.  The
alternative -- rewriting `derivepassphrase` for another command-line
framework, or rolling our own command-line interface from scratch --
seems even more expensive/wasteful than this solution.

src/derivepassphrase/cli.py 2173aca..db00108
tests/test_derivepassphrase_cli.py b43b4de..2bb979c

src/derivepassphrase/cli.py

Zeige Datei @ 10c7604

...	...	@@ -61,7 +61,69 @@ _EMPTY_SELECTION = 'Empty selection'
61	61	# =========
62	62
63	63
64		-@click.command(
	64	+class _DefaultToVaultGroup(click.Group):
	65	+ """A helper class to implement the default-to-"vault"-subcommand behavior.
	66	+
	67	+ Modifies internal [`click.MultiCommand`][] methods, and thus is both
	68	+ an implementation detail and a kludge.
	69	+
	70	+ """
	71	+
	72	+ def resolve_command(
	73	+ self, ctx: click.Context, args: list[str]
	74	+ ) -> tuple[str \| None, click.Command \| None, list[str]]:
	75	+ """Resolve a command, but default to "vault" instead of erroring out.
	76	+
	77	+ Based on code from click 8.1, which appears to be essentially
	78	+ untouched since at least click 3.2.
	79	+
	80	+ """
	81	+ cmd_name = click.utils.make_str(args[0])
	82	+
	83	+ # ORIGINAL COMMENT
	84	+ # Get the command
	85	+ cmd = self.get_command(ctx, cmd_name)
	86	+
	87	+ # ORIGINAL COMMENT
	88	+ # If we can't find the command but there is a normalization
	89	+ # function available, we try with that one.
	90	+ if ( # pragma: no cover
	91	+ cmd is None and ctx.token_normalize_func is not None
	92	+ ):
	93	+ cmd_name = ctx.token_normalize_func(cmd_name)
	94	+ cmd = self.get_command(ctx, cmd_name)
	95	+
	96	+ # ORIGINAL COMMENT
	97	+ # If we don't find the command we want to show an error message
	98	+ # to the user that it was not provided. However, there is
	99	+ # something else we should do: if the first argument looks like
	100	+ # an option we want to kick off parsing again for arguments to
	101	+ # resolve things like --help which now should go to the main
	102	+ # place.
	103	+ if cmd is None and not ctx.resilient_parsing:
	104	+ if click.parser.split_opt(cmd_name)[0]:
	105	+ self.parse_args(ctx, ctx.args)
	106	+ # Instead of calling ctx.fail here, default to "vault", and
	107	+ # issue a deprecation warning.
	108	+ click.echo(
	109	+ (
	110	+ f'{PROG_NAME}: Deprecation warning: A subcommand will be '
	111	+ f'required in v1.0. See --help for available subcommands.'
	112	+ ),
	113	+ err=True,
	114	+ )
	115	+ click.echo(
	116	+ f'{PROG_NAME}: Warning: Defaulting to subcommand "vault".',
	117	+ err=True,
	118	+ )
	119	+ cmd_name = 'vault'
	120	+ cmd = self.get_command(ctx, cmd_name)
	121	+ assert cmd is not None, 'Mandatory subcommand "vault" missing!'
	122	+ args = [cmd_name, *args]
	123	+ return cmd_name if cmd else None, cmd, args[1:] # noqa: DOC201
	124	+
	125	+
	126	+@click.group(
65	127	context_settings={
66	128	'help_option_names': ['-h', '--help'],
67	129	'ignore_unknown_options': True,
...	...	@@ -73,13 +135,12 @@ _EMPTY_SELECTION = 'Empty selection'
73	135	`~/.derivepassphrase` on UNIX-like systems and
74	136	`C:\Users\<user>\AppData\Roaming\Derivepassphrase` on Windows.
75	137	""",
	138	+ invoke_without_command=True,
	139	+ cls=_DefaultToVaultGroup,
76	140	)
77	141	@click.version_option(version=dpp.__version__, prog_name=PROG_NAME)
78		-@click.argument('subcommand_args', nargs=-1, type=click.UNPROCESSED)
79		-def derivepassphrase(
80		- *,
81		- subcommand_args: list[str],
82		-) -> None:
	142	+@click.pass_context
	143	+def derivepassphrase(ctx: click.Context, /) -> None:
83	144	"""Derive a strong passphrase, deterministically, from a master secret.
84	145
85	146	Using a master secret, derive a passphrase for a named service,
...	...	@@ -109,13 +170,7 @@ def derivepassphrase(
109	170	[CLICK]: https://pypi.org/package/click/
110	171
111	172	""" # noqa: D301
112		- if subcommand_args and subcommand_args[0] == 'export':
113		- return derivepassphrase_export.main(
114		- args=subcommand_args[1:],
115		- prog_name=f'{PROG_NAME} export',
116		- standalone_mode=False,
117		- )
118		- if not (subcommand_args and subcommand_args[0] == 'vault'):
	173	+ if ctx.invoked_subcommand is None:
119	174	click.echo(
120	175	(
121	176	f'{PROG_NAME}: Deprecation warning: A subcommand will be '
...	...	@@ -127,32 +182,33 @@ def derivepassphrase(
127	182	f'{PROG_NAME}: Warning: Defaulting to subcommand "vault".',
128	183	err=True,
129	184	)
130		- else:
131		- subcommand_args = subcommand_args[1:]
132		- return derivepassphrase_vault.main(
133		- args=subcommand_args,
134		- prog_name=f'{PROG_NAME} vault',
135		- standalone_mode=False,
	185	+ # See definition of click.Group.invoke, non-chained case.
	186	+ with ctx:
	187	+ sub_ctx = derivepassphrase_vault.make_context(
	188	+ 'vault', ctx.args, parent=ctx
136	189	)
	190	+ with sub_ctx:
	191	+ return derivepassphrase_vault.invoke(sub_ctx)
	192	+ return None
137	193
138	194
139	195	# Exporter
140	196	# ========
141	197
142	198
143		-@click.command(
	199	+@derivepassphrase.group(
	200	+ 'export',
144	201	context_settings={
145	202	'help_option_names': ['-h', '--help'],
146	203	'ignore_unknown_options': True,
147	204	'allow_interspersed_args': False,
148		- }
	205	+ },
	206	+ invoke_without_command=True,
	207	+ cls=_DefaultToVaultGroup,
149	208	)
150	209	@click.version_option(version=dpp.__version__, prog_name=PROG_NAME)
151		-@click.argument('subcommand_args', nargs=-1, type=click.UNPROCESSED)
152		-def derivepassphrase_export(
153		- *,
154		- subcommand_args: list[str],
155		-) -> None:
	210	+@click.pass_context
	211	+def derivepassphrase_export(ctx: click.Context, /) -> None:
156	212	"""Export a foreign configuration to standard output.
157	213
158	214	Read a foreign system configuration, extract all information from
...	...	@@ -174,7 +230,7 @@ def derivepassphrase_export(
174	230	[CLICK]: https://pypi.org/package/click/
175	231
176	232	""" # noqa: D301
177		- if not (subcommand_args and subcommand_args[0] == 'vault'):
	233	+ if ctx.invoked_subcommand is None:
178	234	click.echo(
179	235	(
180	236	f'{PROG_NAME}: Deprecation warning: A subcommand will be '
...	...	@@ -186,13 +242,17 @@ def derivepassphrase_export(
186	242	f'{PROG_NAME}: Warning: Defaulting to subcommand "vault".',
187	243	err=True,
188	244	)
189		- else:
190		- subcommand_args = subcommand_args[1:]
191		- return derivepassphrase_export_vault.main(
192		- args=subcommand_args,
193		- prog_name=f'{PROG_NAME} export vault',
194		- standalone_mode=False,
	245	+ # See definition of click.Group.invoke, non-chained case.
	246	+ with ctx:
	247	+ sub_ctx = derivepassphrase_export_vault.make_context(
	248	+ 'vault', ctx.args, parent=ctx
195	249	)
	250	+ # Constructing the subcontext above will usually already
	251	+ # lead to a click.UsageError, so this block typically won't
	252	+ # actually be called.
	253	+ with sub_ctx: # pragma: no cover
	254	+ return derivepassphrase_export_vault.invoke(sub_ctx)
	255	+ return None
196	256
197	257
198	258	def _load_data(
...	...	@@ -234,7 +294,8 @@ def _load_data(
234	294	assert_never(fmt)
235	295
236	296
237		-@click.command(
	297	+@derivepassphrase_export.command(
	298	+ 'vault',
238	299	context_settings={'help_option_names': ['-h', '--help']},
239	300	)
240	301	@click.option(
...	...	@@ -919,9 +980,8 @@ DEFAULT_NOTES_TEMPLATE = """\
919	980	DEFAULT_NOTES_MARKER = '# - - - - - >8 - - - - -'
920	981
921	982
922		-@click.command(
923		- # 'vault',
924		- # help="derivation scheme compatible with James Coglan's vault(1)",
	983	+@derivepassphrase.command(
	984	+ 'vault',
925	985	context_settings={'help_option_names': ['-h', '--help']},
926	986	cls=CommandWithHelpGroups,
927	987	epilog=r"""

tests/test_derivepassphrase_cli.py

Zeige Datei @ 10c7604

@@ -2040,6 +2040,30 @@ class TestCLITransition:
         )
         assert json.loads(result.output) == tests.VAULT_V03_CONFIG_DATA
 
+    def test_201_forward_export_vault_empty_commandline(
+        self,
+        monkeypatch: pytest.MonkeyPatch,
+        caplog: pytest.LogCaptureFixture,
+    ) -> None:
+        pytest.importorskip('cryptography', minversion='38.0')
+        runner = click.testing.CliRunner(mix_stderr=False)
+        with tests.isolated_config(
+            monkeypatch=monkeypatch,
+            runner=runner,
+        ):
+            _result = runner.invoke(
+                cli.derivepassphrase,
+                ['export'],
+            )
+        result = tests.ReadableResult.parse(_result)
+        assert result.stderr.startswith(f"""\
+{cli.PROG_NAME}: Deprecation warning: A subcommand will be required in v1.0. See --help for available subcommands.
+{cli.PROG_NAME}: Warning: Defaulting to subcommand "vault".
+""")  # noqa: E501
+        assert result.error_exit(
+            error="Missing argument 'PATH'"
+        ), 'expected error exit and known error type'
+
     @pytest.mark.parametrize(
         'charset_name', ['lower', 'upper', 'number', 'space', 'dash', 'symbol']
     )
@@ -2074,6 +2098,31 @@ class TestCLITransition:
                 c not in result.output
             ), f'derived password contains forbidden character {c!r}'
 
+    def test_211_forward_vault_empty_command_line(
+        self,
+        monkeypatch: pytest.MonkeyPatch,
+        caplog: pytest.LogCaptureFixture,
+    ) -> None:
+        runner = click.testing.CliRunner(mix_stderr=False)
+        with tests.isolated_config(
+            monkeypatch=monkeypatch,
+            runner=runner,
+        ):
+            _result = runner.invoke(
+                cli.derivepassphrase,
+                [],
+                input=DUMMY_PASSPHRASE,
+                catch_exceptions=False,
+            )
+            result = tests.ReadableResult.parse(_result)
+        assert result.stderr.startswith(f"""\
+{cli.PROG_NAME}: Deprecation warning: A subcommand will be required in v1.0. See --help for available subcommands.
+{cli.PROG_NAME}: Warning: Defaulting to subcommand "vault".
+""")  # noqa: E501
+        assert result.error_exit(
+            error='SERVICE is required'
+        ), 'expected error exit and known error type'
+
     def test_300_export_using_old_config_file(
         self,
         monkeypatch: pytest.MonkeyPatch,


...	...	@@ -2040,6 +2040,30 @@ class TestCLITransition:
2040	2040	)
2041	2041	assert json.loads(result.output) == tests.VAULT_V03_CONFIG_DATA
2042	2042
	2043	+ def test_201_forward_export_vault_empty_commandline(
	2044	+ self,
	2045	+ monkeypatch: pytest.MonkeyPatch,
	2046	+ caplog: pytest.LogCaptureFixture,
	2047	+ ) -> None:
	2048	+ pytest.importorskip('cryptography', minversion='38.0')
	2049	+ runner = click.testing.CliRunner(mix_stderr=False)
	2050	+ with tests.isolated_config(
	2051	+ monkeypatch=monkeypatch,
	2052	+ runner=runner,
	2053	+ ):
	2054	+ _result = runner.invoke(
	2055	+ cli.derivepassphrase,
	2056	+ ['export'],
	2057	+ )
	2058	+ result = tests.ReadableResult.parse(_result)
	2059	+ assert result.stderr.startswith(f"""\
	2060	+{cli.PROG_NAME}: Deprecation warning: A subcommand will be required in v1.0. See --help for available subcommands.
	2061	+{cli.PROG_NAME}: Warning: Defaulting to subcommand "vault".
	2062	+""") # noqa: E501
	2063	+ assert result.error_exit(
	2064	+ error="Missing argument 'PATH'"
	2065	+ ), 'expected error exit and known error type'
	2066	+
2043	2067	@pytest.mark.parametrize(
2044	2068	'charset_name', ['lower', 'upper', 'number', 'space', 'dash', 'symbol']
2045	2069	)
...	...	@@ -2074,6 +2098,31 @@ class TestCLITransition:
2074	2098	c not in result.output
2075	2099	), f'derived password contains forbidden character {c!r}'
2076	2100
	2101	+ def test_211_forward_vault_empty_command_line(
	2102	+ self,
	2103	+ monkeypatch: pytest.MonkeyPatch,
	2104	+ caplog: pytest.LogCaptureFixture,
	2105	+ ) -> None:
	2106	+ runner = click.testing.CliRunner(mix_stderr=False)
	2107	+ with tests.isolated_config(
	2108	+ monkeypatch=monkeypatch,
	2109	+ runner=runner,
	2110	+ ):
	2111	+ _result = runner.invoke(
	2112	+ cli.derivepassphrase,
	2113	+ [],
	2114	+ input=DUMMY_PASSPHRASE,
	2115	+ catch_exceptions=False,
	2116	+ )
	2117	+ result = tests.ReadableResult.parse(_result)
	2118	+ assert result.stderr.startswith(f"""\
	2119	+{cli.PROG_NAME}: Deprecation warning: A subcommand will be required in v1.0. See --help for available subcommands.
	2120	+{cli.PROG_NAME}: Warning: Defaulting to subcommand "vault".
	2121	+""") # noqa: E501
	2122	+ assert result.error_exit(
	2123	+ error='SERVICE is required'
	2124	+ ), 'expected error exit and known error type'
	2125	+
2077	2126	def test_300_export_using_old_config_file(
2078	2127	self,
2079	2128	monkeypatch: pytest.MonkeyPatch,
2080	2129