Warn the user when `derivepassphrase vault --notes` is ineffective
Marco Ricci commited on 2025-02-06 13:16:37
Zeige 4 geänderte Dateien mit 92 Einfügungen und 0 Löschungen.
In 7b0f4e121a5e688b59abad73b8b60bacfc7d02ed, we adjusted `derivepassphrase` to only act upon `--notes` if `--config` is also given, for compatibility with vault(1). However, this is incompatible with previous versions of `derivepassphrase`, and potentially confusing even to vault(1) users. As such, `derivepassphrase vault` now emits a warning if it is called with `--notes` but without `--config`.
- pyproject.toml 20c9a0e..bf229c2
- src/derivepassphrase/_internals/cli_messages.py 1c2aaa0..d03b9ae
- src/derivepassphrase/cli.py 33a56e7..e925cda
- tests/test_derivepassphrase_cli.py dbe0728..2822fc2
| ... | ... |
@@ -1740,6 +1740,13 @@ class InfoMsgTemplate(enum.Enum): |
| 1740 | 1740 |
class WarnMsgTemplate(enum.Enum): |
| 1741 | 1741 |
"""Warning messages for the `derivepassphrase` command-line.""" |
| 1742 | 1742 |
|
| 1743 |
+ EDITING_NOTES_BUT_NOT_STORING_CONFIG = commented( |
|
| 1744 |
+ '', |
|
| 1745 |
+ )( |
|
| 1746 |
+ 'Warning message', |
|
| 1747 |
+ 'Specifying --notes without --config is ineffective. ' |
|
| 1748 |
+ 'No notes will be edited.', |
|
| 1749 |
+ ) |
|
| 1743 | 1750 |
EMPTY_SERVICE_NOT_SUPPORTED = commented( |
| 1744 | 1751 |
'', |
| 1745 | 1752 |
)( |
| ... | ... |
@@ -987,6 +987,15 @@ def derivepassphrase_vault( # noqa: C901,PLR0912,PLR0913,PLR0914,PLR0915 |
| 987 | 987 |
extra={'color': ctx.color},
|
| 988 | 988 |
) |
| 989 | 989 |
|
| 990 |
+ if edit_notes and not store_config_only: |
|
| 991 |
+ logger.warning( |
|
| 992 |
+ _msg.TranslatedString( |
|
| 993 |
+ _msg.WarnMsgTemplate.EDITING_NOTES_BUT_NOT_STORING_CONFIG, |
|
| 994 |
+ service_metavar=service_metavar, |
|
| 995 |
+ ), |
|
| 996 |
+ extra={'color': ctx.color},
|
|
| 997 |
+ ) |
|
| 998 |
+ |
|
| 990 | 999 |
if delete_service_settings: |
| 991 | 1000 |
assert service is not None |
| 992 | 1001 |
configuration = get_config() |
| ... | ... |
@@ -2763,6 +2763,81 @@ class TestCLI: |
| 2763 | 2763 |
'services': {},
|
| 2764 | 2764 |
} |
| 2765 | 2765 |
|
| 2766 |
+ @hypothesis.settings( |
|
| 2767 |
+ suppress_health_check=[ |
|
| 2768 |
+ *hypothesis.settings().suppress_health_check, |
|
| 2769 |
+ hypothesis.HealthCheck.function_scoped_fixture, |
|
| 2770 |
+ ], |
|
| 2771 |
+ ) |
|
| 2772 |
+ @hypothesis.given( |
|
| 2773 |
+ notes=strategies.text( |
|
| 2774 |
+ strategies.characters( |
|
| 2775 |
+ min_codepoint=32, max_codepoint=126, include_characters='\n' |
|
| 2776 |
+ ), |
|
| 2777 |
+ max_size=512, |
|
| 2778 |
+ ), |
|
| 2779 |
+ ) |
|
| 2780 |
+ def test_223b_edit_notes_fail_config_option_missing( |
|
| 2781 |
+ self, |
|
| 2782 |
+ caplog: pytest.LogCaptureFixture, |
|
| 2783 |
+ notes: str, |
|
| 2784 |
+ ) -> None: |
|
| 2785 |
+ """Editing notes fails (and warns) if `--config` is missing.""" |
|
| 2786 |
+ maybe_notes = {'notes': notes.strip()} if notes.strip() else {}
|
|
| 2787 |
+ vault_config = {
|
|
| 2788 |
+ 'global': {'phrase': DUMMY_PASSPHRASE},
|
|
| 2789 |
+ 'services': {
|
|
| 2790 |
+ DUMMY_SERVICE: {**maybe_notes, **DUMMY_CONFIG_SETTINGS}
|
|
| 2791 |
+ }, |
|
| 2792 |
+ } |
|
| 2793 |
+ # Reset caplog between hypothesis runs. |
|
| 2794 |
+ caplog.clear() |
|
| 2795 |
+ runner = click.testing.CliRunner(mix_stderr=False) |
|
| 2796 |
+ # TODO(the-13th-letter): Rewrite using parenthesized |
|
| 2797 |
+ # with-statements. |
|
| 2798 |
+ # https://the13thletter.info/derivepassphrase/latest/pycompatibility/#after-eol-py3.9 |
|
| 2799 |
+ with contextlib.ExitStack() as stack: |
|
| 2800 |
+ monkeypatch = stack.enter_context(pytest.MonkeyPatch.context()) |
|
| 2801 |
+ stack.enter_context( |
|
| 2802 |
+ tests.isolated_vault_config( |
|
| 2803 |
+ monkeypatch=monkeypatch, |
|
| 2804 |
+ runner=runner, |
|
| 2805 |
+ vault_config=vault_config, |
|
| 2806 |
+ ) |
|
| 2807 |
+ ) |
|
| 2808 |
+ EDIT_ATTEMPTED = 'edit attempted!' # noqa: N806 |
|
| 2809 |
+ |
|
| 2810 |
+ def raiser(*_args: Any, **_kwargs: Any) -> NoReturn: |
|
| 2811 |
+ pytest.fail(EDIT_ATTEMPTED) |
|
| 2812 |
+ |
|
| 2813 |
+ monkeypatch.setattr(click, 'edit', raiser) |
|
| 2814 |
+ result_ = runner.invoke( |
|
| 2815 |
+ cli.derivepassphrase_vault, |
|
| 2816 |
+ ['--notes', '--', DUMMY_SERVICE], |
|
| 2817 |
+ catch_exceptions=False, |
|
| 2818 |
+ ) |
|
| 2819 |
+ result = tests.ReadableResult.parse(result_) |
|
| 2820 |
+ assert result.clean_exit( |
|
| 2821 |
+ output=DUMMY_RESULT_PASSPHRASE.decode('ascii')
|
|
| 2822 |
+ ), 'expected clean exit' |
|
| 2823 |
+ assert result.stderr |
|
| 2824 |
+ assert notes.strip() in result.stderr |
|
| 2825 |
+ assert all( |
|
| 2826 |
+ is_warning_line(line) |
|
| 2827 |
+ for line in result.stderr.splitlines(True) |
|
| 2828 |
+ if line.startswith(f'{cli.PROG_NAME}: ')
|
|
| 2829 |
+ ) |
|
| 2830 |
+ assert tests.warning_emitted( |
|
| 2831 |
+ 'Specifying --notes without --config is ineffective. ' |
|
| 2832 |
+ 'No notes will be edited.', |
|
| 2833 |
+ caplog.record_tuples, |
|
| 2834 |
+ ), 'expected known warning message in stderr' |
|
| 2835 |
+ with cli_helpers.config_filename(subsystem='vault').open( |
|
| 2836 |
+ encoding='UTF-8' |
|
| 2837 |
+ ) as infile: |
|
| 2838 |
+ config = json.load(infile) |
|
| 2839 |
+ assert config == vault_config |
|
| 2840 |
+ |
|
| 2766 | 2841 |
@Parametrize.CONFIG_EDITING_VIA_CONFIG_FLAG |
| 2767 | 2842 |
def test_224_store_config_good( |
| 2768 | 2843 |
self, |
| 2769 | 2844 |