Remove `click` handling of command-line "path" arguments
Marco Ricci

Marco Ricci commited on 2024-09-01 18:08:20
Zeige 2 geänderte Dateien mit 5 Einfügungen und 4 Löschungen.


Stop using the `click` facilities to run checks on command-line
arguments or option values that are to be treated as paths; treat them
like normal strings instead.  `click`'s path checking functionality
misdiagnoses its own failure cases, is racy, and does not document its
own raciness.

Instead, use `click`'s `open_file` function to handle the standard
streams (an actual useful feature), and use bog-standard operating
system calls for everything else.  As an added bonus, this makes writing
unit tests much easier, because we no longer have to work around
`click`'s functionality and can use simpler counterexamples/failure
cases.

This change was originally prompted by a silently failing test, which
did not test the expected error message because it was subject to
translation, so `click` could silently sidestep the whole code under
test.  We now test for the operating system-provided error message
directly, via standard operating system facilities that are integrated
into the translation/locale system.
... ...
@@ -737,7 +737,6 @@ DEFAULT_NOTES_MARKER = '# - - - - - >8 - - - - -'
737 737
     '--export',
738 738
     'export_settings',
739 739
     metavar='PATH',
740
-    type=click.Path(file_okay=True, allow_dash=True, exists=False),
741 740
     help='export all saved settings into file PATH',
742 741
     cls=StorageManagementOption,
743 742
 )
... ...
@@ -746,7 +745,6 @@ DEFAULT_NOTES_MARKER = '# - - - - - >8 - - - - -'
746 745
     '--import',
747 746
     'import_settings',
748 747
     metavar='PATH',
749
-    type=click.Path(file_okay=True, allow_dash=True, exists=False),
750 748
     help='import saved settings from file PATH',
751 749
     cls=StorageManagementOption,
752 750
 )
... ...
@@ -5,6 +5,7 @@
5 5
 from __future__ import annotations
6 6
 
7 7
 import contextlib
8
+import errno
8 9
 import json
9 10
 import os
10 11
 import shutil
... ...
@@ -659,8 +660,10 @@ class TestCLI:
659 660
             assert (
660 661
                 result.stderr_bytes
661 662
             ), 'program did not print any error message'
662
-            # Don't test the actual error message, because it is subject to
663
-            # locale settings.  TODO: find a way anyway.
663
+            assert (
664
+                os.strerror(errno.EISDIR).encode('utf-8')
665
+                in result.stderr_bytes
666
+            ), 'program did not print the expected error message'
664 667
 
665 668
     def test_214_export_settings_no_stored_settings(
666 669
         self,
667 670