From 0009471f67f5a00f03bcaa925d0d92988f7c85c4 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 27 Apr 2023 18:46:13 -0700 Subject: [PATCH 01/49] start work on completion --- borgmatic/commands/arguments.py | 6 ++++++ borgmatic/commands/borgmatic.py | 3 +++ borgmatic/commands/completion.py | 22 ++++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 61b547690..c71988561 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -207,6 +207,12 @@ def make_parsers(): action='store_true', help='Show bash completion script and exit', ) + global_group.add_argument( + '--fish-completion', + default=False, + action='store_true', + help='Show fish completion script and exit', + ) global_group.add_argument( '--version', dest='version', diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 999e9d8e1..d5fedba74 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -715,6 +715,9 @@ def main(): # pragma: no cover if global_arguments.bash_completion: print(borgmatic.commands.completion.bash_completion()) sys.exit(0) + if global_arguments.fish_completion: + print(borgmatic.commands.completion.fish_completion()) + sys.exit(0) config_filenames = tuple(collect.collect_config_filenames(global_arguments.config_paths)) configs, parse_logs = load_configurations( diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 1fc976bc6..4f262ddd1 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -55,3 +55,25 @@ def bash_completion(): '\ncomplete -o bashdefault -o default -F complete_borgmatic borgmatic', ) ) + +def fish_completion(): + ''' + Return a fish completion script for the borgmatic command. Produce this by introspecting + borgmatic's command-line argument parsers. + ''' + top_level_parser, subparsers = arguments.make_parsers() + global_flags = parser_flags(top_level_parser) + actions = ' '.join(subparsers.choices.keys()) + + # Avert your eyes. + return '\n'.join( + ( + 'function __borgmatic_check_version', + ' set this_script (cat "$BASH_SOURCE" 2> /dev/null)', + ' set installed_script (borgmatic --bash-completion 2> /dev/null)', + ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ]', + f' echo "{UPGRADE_MESSAGE}"', + ' end', + 'end', + 'function __borgmatic_complete', + )) From 28b152aedd5e460cb16009b339781789650e40e9 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 27 Apr 2023 19:31:42 -0700 Subject: [PATCH 02/49] make upgrade message a template --- borgmatic/commands/completion.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 4f262ddd1..13210b025 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -1,12 +1,13 @@ from borgmatic.commands import arguments -UPGRADE_MESSAGE = ''' -Your bash completions script is from a different version of borgmatic than is +def upgrade_message(language: str, upgrade_command: str, completion_file: str): + return f''' +Your {language} completions script is from a different version of borgmatic than is currently installed. Please upgrade your script so your completions match the command-line flags in your installed borgmatic! Try this to upgrade: - sudo sh -c "borgmatic --bash-completion > $BASH_SOURCE" - source $BASH_SOURCE + {upgrade_command} + source {completion_file} ''' @@ -34,7 +35,7 @@ def bash_completion(): ' local this_script="$(cat "$BASH_SOURCE" 2> /dev/null)"', ' local installed_script="$(borgmatic --bash-completion 2> /dev/null)"', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ];' - f' then cat << EOF\n{UPGRADE_MESSAGE}\nEOF', + ' then cat << EOF\n{}\nEOF'.format(upgrade_message('bash', 'sudo sh -c "borgmatic --bash-completion > $BASH_SOURCE"', '$BASH_SOURCE')), ' fi', '}', 'complete_borgmatic() {', @@ -69,11 +70,11 @@ def fish_completion(): return '\n'.join( ( 'function __borgmatic_check_version', - ' set this_script (cat "$BASH_SOURCE" 2> /dev/null)', - ' set installed_script (borgmatic --bash-completion 2> /dev/null)', + ' set this_script (status current-filename)', + ' set installed_script (borgmatic --fish-completion 2> /dev/null)', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ]', - f' echo "{UPGRADE_MESSAGE}"', + ' echo "{}"'.format(upgrade_message('fish', 'borgmatic --fish-completion | sudo tee (status current-filename)', '(status current-filename)')), ' end', 'end', - 'function __borgmatic_complete', + # 'function __borgmatic_complete', )) From 5678f3a96e9731e3ff747de13440f53aa1572f5c Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 27 Apr 2023 19:44:11 -0700 Subject: [PATCH 03/49] basic working version --- borgmatic/commands/completion.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 13210b025..de0991c97 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -1,4 +1,5 @@ from borgmatic.commands import arguments +import shlex def upgrade_message(language: str, upgrade_command: str, completion_file: str): return f''' @@ -76,5 +77,12 @@ def fish_completion(): ' echo "{}"'.format(upgrade_message('fish', 'borgmatic --fish-completion | sudo tee (status current-filename)', '(status current-filename)')), ' end', 'end', - # 'function __borgmatic_complete', - )) + ) + tuple( + '''complete -c borgmatic -n '__borgmatic_check_version' -a '%s' -d %s -f''' + % (action, shlex.quote(subparser.description)) + for action, subparser in subparsers.choices.items() + ) + ( + 'complete -c borgmatic -a "%s" -d "borgmatic actions" -f' % actions, + 'complete -c borgmatic -a "%s" -d "borgmatic global flags" -f' % global_flags, + ) + ) From 25b3db72a0d9bb47df8f44ad86c5a705eb8104ee Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 27 Apr 2023 19:58:22 -0700 Subject: [PATCH 04/49] make more precise, fix the version check fn --- borgmatic/commands/completion.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index de0991c97..da62ccccf 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -64,25 +64,29 @@ def fish_completion(): borgmatic's command-line argument parsers. ''' top_level_parser, subparsers = arguments.make_parsers() - global_flags = parser_flags(top_level_parser) actions = ' '.join(subparsers.choices.keys()) # Avert your eyes. return '\n'.join( ( 'function __borgmatic_check_version', - ' set this_script (status current-filename)', + ' set this_script (cat (status current-filename) 2> /dev/null)', ' set installed_script (borgmatic --fish-completion 2> /dev/null)', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ]', ' echo "{}"'.format(upgrade_message('fish', 'borgmatic --fish-completion | sudo tee (status current-filename)', '(status current-filename)')), ' end', 'end', + '__borgmatic_check_version &', ) + tuple( - '''complete -c borgmatic -n '__borgmatic_check_version' -a '%s' -d %s -f''' + '''complete -c borgmatic -a '%s' -d %s -f''' % (action, shlex.quote(subparser.description)) for action, subparser in subparsers.choices.items() ) + ( 'complete -c borgmatic -a "%s" -d "borgmatic actions" -f' % actions, - 'complete -c borgmatic -a "%s" -d "borgmatic global flags" -f' % global_flags, + ) + tuple( + '''complete -c borgmatic -a '%s' -d %s -f''' + % (option, shlex.quote(action.help)) + for action in top_level_parser._actions + for option in action.option_strings ) ) From 8060586d8b5088c2d96d91a0a34c79eb2bf8763b Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 27 Apr 2023 20:05:17 -0700 Subject: [PATCH 05/49] fix the script and drop unneeded options --- borgmatic/commands/completion.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index da62ccccf..aadf85e25 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -64,16 +64,16 @@ def fish_completion(): borgmatic's command-line argument parsers. ''' top_level_parser, subparsers = arguments.make_parsers() - actions = ' '.join(subparsers.choices.keys()) # Avert your eyes. return '\n'.join( ( 'function __borgmatic_check_version', - ' set this_script (cat (status current-filename) 2> /dev/null)', + ' set this_filename (status current-filename)', + ' set this_script (cat $this_filename 2> /dev/null)', ' set installed_script (borgmatic --fish-completion 2> /dev/null)', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ]', - ' echo "{}"'.format(upgrade_message('fish', 'borgmatic --fish-completion | sudo tee (status current-filename)', '(status current-filename)')), + ' echo "{}"'.format(upgrade_message('fish', 'borgmatic --fish-completion | sudo tee $this_filename', '$this_filename')), ' end', 'end', '__borgmatic_check_version &', @@ -81,8 +81,6 @@ def fish_completion(): '''complete -c borgmatic -a '%s' -d %s -f''' % (action, shlex.quote(subparser.description)) for action, subparser in subparsers.choices.items() - ) + ( - 'complete -c borgmatic -a "%s" -d "borgmatic actions" -f' % actions, ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f''' % (option, shlex.quote(action.help)) From 412d18f2183292c0edd46cd8566aab0328eade50 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 27 Apr 2023 21:31:53 -0700 Subject: [PATCH 06/49] show sub options --- borgmatic/commands/completion.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index aadf85e25..5555c42bb 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -79,12 +79,18 @@ def fish_completion(): '__borgmatic_check_version &', ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f''' - % (action, shlex.quote(subparser.description)) - for action, subparser in subparsers.choices.items() + % (actionStr, shlex.quote(subparser.description)) + for actionStr, subparser in subparsers.choices.items() ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f''' % (option, shlex.quote(action.help)) for action in top_level_parser._actions for option in action.option_strings + ) + tuple( + '''complete -c borgmatic -a '%s' -d %s -f -n "__fish_seen_subcommand_from %s"''' + % (option, shlex.quote(action.help), actionStr) + for actionStr, subparser in subparsers.choices.items() + for action in subparser._actions + for option in action.option_strings ) ) From 2e658cfa5687b90d867ef69baa8be94cc29a802d Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 27 Apr 2023 21:57:50 -0700 Subject: [PATCH 07/49] only allow one parser --- borgmatic/commands/completion.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 5555c42bb..225fffa7f 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -65,6 +65,8 @@ def fish_completion(): ''' top_level_parser, subparsers = arguments.make_parsers() + all_subparsers = ' '.join(action for action in subparsers.choices.keys()) + # Avert your eyes. return '\n'.join( ( @@ -78,8 +80,8 @@ def fish_completion(): 'end', '__borgmatic_check_version &', ) + tuple( - '''complete -c borgmatic -a '%s' -d %s -f''' - % (actionStr, shlex.quote(subparser.description)) + '''complete -c borgmatic -a '%s' -d %s -f -n "not __fish_seen_subcommand_from %s"''' + % (actionStr, shlex.quote(subparser.description), all_subparsers) for actionStr, subparser in subparsers.choices.items() ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f''' From d265b6ed6f316a16e9c945829a8da1596841f1e2 Mon Sep 17 00:00:00 2001 From: Isaac Date: Fri, 28 Apr 2023 11:57:16 -0700 Subject: [PATCH 08/49] add comments in generated files --- borgmatic/commands/completion.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 225fffa7f..56109f37a 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -79,15 +79,21 @@ def fish_completion(): ' end', 'end', '__borgmatic_check_version &', + ) + ( + '# subparser completions', ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f -n "not __fish_seen_subcommand_from %s"''' % (actionStr, shlex.quote(subparser.description), all_subparsers) for actionStr, subparser in subparsers.choices.items() + ) + ( + '# global flags', ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f''' % (option, shlex.quote(action.help)) for action in top_level_parser._actions for option in action.option_strings + ) + ( + '# subparser flags', ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f -n "__fish_seen_subcommand_from %s"''' % (option, shlex.quote(action.help), actionStr) From 23f478ce7454fc715e23dc695abd38884854c0b8 Mon Sep 17 00:00:00 2001 From: Isaac Date: Fri, 28 Apr 2023 12:13:08 -0700 Subject: [PATCH 09/49] use less completion lines --- borgmatic/commands/completion.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 56109f37a..a256ad510 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -80,25 +80,24 @@ def fish_completion(): 'end', '__borgmatic_check_version &', ) + ( - '# subparser completions', + '\n# subparser completions', ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f -n "not __fish_seen_subcommand_from %s"''' % (actionStr, shlex.quote(subparser.description), all_subparsers) for actionStr, subparser in subparsers.choices.items() ) + ( - '# global flags', + '\n# global flags', ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f''' % (option, shlex.quote(action.help)) for action in top_level_parser._actions for option in action.option_strings ) + ( - '# subparser flags', + '\n# subparser flags', ) + tuple( - '''complete -c borgmatic -a '%s' -d %s -f -n "__fish_seen_subcommand_from %s"''' - % (option, shlex.quote(action.help), actionStr) + '''complete -c borgmatic -a '%s' -d %s -n "__fish_seen_subcommand_from %s" -f''' + % (' '.join(action.option_strings), shlex.quote(action.help), actionStr) for actionStr, subparser in subparsers.choices.items() for action in subparser._actions - for option in action.option_strings ) ) From 9c77ebb01600b7caa093a4818b3fed662df28183 Mon Sep 17 00:00:00 2001 From: Isaac Date: Fri, 28 Apr 2023 12:15:01 -0700 Subject: [PATCH 10/49] continue deduping --- borgmatic/commands/completion.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index a256ad510..e711b789d 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -89,9 +89,8 @@ def fish_completion(): '\n# global flags', ) + tuple( '''complete -c borgmatic -a '%s' -d %s -f''' - % (option, shlex.quote(action.help)) + % (' '.join(action.option_strings), shlex.quote(action.help)) for action in top_level_parser._actions - for option in action.option_strings ) + ( '\n# subparser flags', ) + tuple( From 98e3a81fcf6d5608ce1ebb7d27fcdefc2910d6d8 Mon Sep 17 00:00:00 2001 From: Isaac Date: Fri, 28 Apr 2023 12:42:26 -0700 Subject: [PATCH 11/49] allow file completions as applicable --- borgmatic/commands/completion.py | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index e711b789d..e62de01c9 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -1,3 +1,4 @@ +from argparse import Action from borgmatic.commands import arguments import shlex @@ -58,6 +59,15 @@ def bash_completion(): ) ) +def build_fish_flags(action: Action): + ''' + Given an argparse.Action instance, return a string containing the fish flags for that action. + ''' + if action.metavar and action.metavar == 'PATH' or action.metavar == 'FILENAME': + return '-r -F' + else: + return '-f' + def fish_completion(): ''' Return a fish completion script for the borgmatic command. Produce this by introspecting @@ -88,14 +98,14 @@ def fish_completion(): ) + ( '\n# global flags', ) + tuple( - '''complete -c borgmatic -a '%s' -d %s -f''' - % (' '.join(action.option_strings), shlex.quote(action.help)) + '''complete -c borgmatic -a '%s' -d %s %s''' + % (' '.join(action.option_strings), shlex.quote(action.help), build_fish_flags(action)) for action in top_level_parser._actions ) + ( '\n# subparser flags', ) + tuple( - '''complete -c borgmatic -a '%s' -d %s -n "__fish_seen_subcommand_from %s" -f''' - % (' '.join(action.option_strings), shlex.quote(action.help), actionStr) + '''complete -c borgmatic -a '%s' -d %s -n "__fish_seen_subcommand_from %s" %s''' + % (' '.join(action.option_strings), shlex.quote(action.help), actionStr, build_fish_flags(action)) for actionStr, subparser in subparsers.choices.items() for action in subparser._actions ) From f7e4024fcae5fc1aa836f0cb90f379df65cdb251 Mon Sep 17 00:00:00 2001 From: Isaac Date: Fri, 28 Apr 2023 14:02:06 -0700 Subject: [PATCH 12/49] add to readme --- docs/how-to/set-up-backups.md | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/docs/how-to/set-up-backups.md b/docs/how-to/set-up-backups.md index be229cb20..098f01399 100644 --- a/docs/how-to/set-up-backups.md +++ b/docs/how-to/set-up-backups.md @@ -334,10 +334,13 @@ Access](https://projects.torsion.org/borgmatic-collective/borgmatic/issues/293). ### Shell completion -borgmatic includes a shell completion script (currently only for Bash) to +borgmatic includes a shell completion script (currently only for Bash and Fish) to support tab-completing borgmatic command-line actions and flags. Depending on -how you installed borgmatic, this may be enabled by default. But if it's not, -start by installing the `bash-completion` Linux package or the +how you installed borgmatic, this may be enabled by default. + +#### Bash + +If completions aren't enabled, start by installing the `bash-completion` Linux package or the [`bash-completion@2`](https://formulae.brew.sh/formula/bash-completion@2) macOS Homebrew formula. Then, install the shell completion script globally: @@ -362,6 +365,14 @@ borgmatic --bash-completion > ~/.local/share/bash-completion/completions/borgmat Finally, restart your shell (`exit` and open a new shell) so the completions take effect. +#### Fish + +To add completions for fish, install the completions file globally: + +```fish +borgmatic --fish-completion | sudo tee /usr/share/fish/vendor_completions.d/borgmatic.fish +source /usr/share/fish/vendor_completions.d/borgmatic.fish +``` ### Colored output From 9ff5ea52409b8e426600996f9a398fcb386aee88 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 13:20:01 -0700 Subject: [PATCH 13/49] add a unit test, fix isort and black --- borgmatic/commands/completion.py | 49 +++++++++++++------ tests/integration/commands/test_completion.py | 4 ++ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index e62de01c9..352dc3654 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -1,6 +1,8 @@ -from argparse import Action -from borgmatic.commands import arguments import shlex +from argparse import Action + +from borgmatic.commands import arguments + def upgrade_message(language: str, upgrade_command: str, completion_file: str): return f''' @@ -37,7 +39,13 @@ def bash_completion(): ' local this_script="$(cat "$BASH_SOURCE" 2> /dev/null)"', ' local installed_script="$(borgmatic --bash-completion 2> /dev/null)"', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ];' - ' then cat << EOF\n{}\nEOF'.format(upgrade_message('bash', 'sudo sh -c "borgmatic --bash-completion > $BASH_SOURCE"', '$BASH_SOURCE')), + ' then cat << EOF\n{}\nEOF'.format( + upgrade_message( + 'bash', + 'sudo sh -c "borgmatic --bash-completion > $BASH_SOURCE"', + '$BASH_SOURCE', + ) + ), ' fi', '}', 'complete_borgmatic() {', @@ -59,6 +67,7 @@ def bash_completion(): ) ) + def build_fish_flags(action: Action): ''' Given an argparse.Action instance, return a string containing the fish flags for that action. @@ -68,6 +77,7 @@ def build_fish_flags(action: Action): else: return '-f' + def fish_completion(): ''' Return a fish completion script for the borgmatic command. Produce this by introspecting @@ -85,27 +95,38 @@ def fish_completion(): ' set this_script (cat $this_filename 2> /dev/null)', ' set installed_script (borgmatic --fish-completion 2> /dev/null)', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ]', - ' echo "{}"'.format(upgrade_message('fish', 'borgmatic --fish-completion | sudo tee $this_filename', '$this_filename')), + ' echo "{}"'.format( + upgrade_message( + 'fish', + 'borgmatic --fish-completion | sudo tee $this_filename', + '$this_filename', + ) + ), ' end', 'end', '__borgmatic_check_version &', - ) + ( - '\n# subparser completions', - ) + tuple( + ) + + ('\n# subparser completions',) + + tuple( '''complete -c borgmatic -a '%s' -d %s -f -n "not __fish_seen_subcommand_from %s"''' % (actionStr, shlex.quote(subparser.description), all_subparsers) for actionStr, subparser in subparsers.choices.items() - ) + ( - '\n# global flags', - ) + tuple( + ) + + ('\n# global flags',) + + tuple( '''complete -c borgmatic -a '%s' -d %s %s''' % (' '.join(action.option_strings), shlex.quote(action.help), build_fish_flags(action)) for action in top_level_parser._actions - ) + ( - '\n# subparser flags', - ) + tuple( + ) + + ('\n# subparser flags',) + + tuple( '''complete -c borgmatic -a '%s' -d %s -n "__fish_seen_subcommand_from %s" %s''' - % (' '.join(action.option_strings), shlex.quote(action.help), actionStr, build_fish_flags(action)) + % ( + ' '.join(action.option_strings), + shlex.quote(action.help), + actionStr, + build_fish_flags(action), + ) for actionStr, subparser in subparsers.choices.items() for action in subparser._actions ) diff --git a/tests/integration/commands/test_completion.py b/tests/integration/commands/test_completion.py index a3b0b9c24..9a118abfc 100644 --- a/tests/integration/commands/test_completion.py +++ b/tests/integration/commands/test_completion.py @@ -3,3 +3,7 @@ from borgmatic.commands import completion as module def test_bash_completion_does_not_raise(): assert module.bash_completion() + + +def test_fish_completion_does_not_raise(): + assert module.fish_completion() From ca689505e57261fb35c6229014b6a0a7e089c87c Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 13:27:00 -0700 Subject: [PATCH 14/49] add e2e fish test --- scripts/run-full-tests | 6 +++--- tests/end-to-end/test_completion.py | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/scripts/run-full-tests b/scripts/run-full-tests index bf26c2124..a7a49a2a5 100755 --- a/scripts/run-full-tests +++ b/scripts/run-full-tests @@ -10,7 +10,7 @@ set -e -if [ -z "$TEST_CONTAINER" ] ; then +if [ -z "$TEST_CONTAINER" ]; then echo "This script is designed to work inside a test container and is not intended to" echo "be run manually. If you're trying to run borgmatic's end-to-end tests, execute" echo "scripts/run-end-to-end-dev-tests instead." @@ -18,14 +18,14 @@ if [ -z "$TEST_CONTAINER" ] ; then fi apk add --no-cache python3 py3-pip borgbackup postgresql-client mariadb-client mongodb-tools \ - py3-ruamel.yaml py3-ruamel.yaml.clib bash sqlite + py3-ruamel.yaml py3-ruamel.yaml.clib bash sqlite fish # If certain dependencies of black are available in this version of Alpine, install them. apk add --no-cache py3-typed-ast py3-regex || true python3 -m pip install --no-cache --upgrade pip==22.2.2 setuptools==64.0.1 pip3 install --ignore-installed tox==3.25.1 export COVERAGE_FILE=/tmp/.coverage -if [ "$1" != "--end-to-end-only" ] ; then +if [ "$1" != "--end-to-end-only" ]; then tox --workdir /tmp/.tox --sitepackages fi diff --git a/tests/end-to-end/test_completion.py b/tests/end-to-end/test_completion.py index e4037ecef..7d6af4cee 100644 --- a/tests/end-to-end/test_completion.py +++ b/tests/end-to-end/test_completion.py @@ -3,3 +3,7 @@ import subprocess def test_bash_completion_runs_without_error(): subprocess.check_call('borgmatic --bash-completion | bash', shell=True) + + +def test_fish_completion_runs_without_error(): + subprocess.check_call('borgmatic --fish-completion | fish', shell=True) From b7fe2a503173614361cd94e946647790f57ec7a5 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 13:27:57 -0700 Subject: [PATCH 15/49] lowercase fish in docs --- docs/how-to/set-up-backups.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how-to/set-up-backups.md b/docs/how-to/set-up-backups.md index 098f01399..de5bf8b92 100644 --- a/docs/how-to/set-up-backups.md +++ b/docs/how-to/set-up-backups.md @@ -365,7 +365,7 @@ borgmatic --bash-completion > ~/.local/share/bash-completion/completions/borgmat Finally, restart your shell (`exit` and open a new shell) so the completions take effect. -#### Fish +#### fish To add completions for fish, install the completions file globally: From 062453af51b7c99401e1508f989a116d01b20a09 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 13:29:25 -0700 Subject: [PATCH 16/49] replace actionStr with action_name --- borgmatic/commands/completion.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 352dc3654..92a221d41 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -109,8 +109,8 @@ def fish_completion(): + ('\n# subparser completions',) + tuple( '''complete -c borgmatic -a '%s' -d %s -f -n "not __fish_seen_subcommand_from %s"''' - % (actionStr, shlex.quote(subparser.description), all_subparsers) - for actionStr, subparser in subparsers.choices.items() + % (action_name, shlex.quote(subparser.description), all_subparsers) + for action_name, subparser in subparsers.choices.items() ) + ('\n# global flags',) + tuple( @@ -124,10 +124,10 @@ def fish_completion(): % ( ' '.join(action.option_strings), shlex.quote(action.help), - actionStr, + action_name, build_fish_flags(action), ) - for actionStr, subparser in subparsers.choices.items() + for action_name, subparser in subparsers.choices.items() for action in subparser._actions ) ) From f04036e4a76dc99b4764f33b067f2d9c7f5f0fbc Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 13:33:21 -0700 Subject: [PATCH 17/49] use fstring to produce completion lines --- borgmatic/commands/completion.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 92a221d41..6a686d2bc 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -108,25 +108,17 @@ def fish_completion(): ) + ('\n# subparser completions',) + tuple( - '''complete -c borgmatic -a '%s' -d %s -f -n "not __fish_seen_subcommand_from %s"''' - % (action_name, shlex.quote(subparser.description), all_subparsers) + f'''complete -c borgmatic -a '{action_name}' -d {shlex.quote(subparser.description)} -f -n "not __fish_seen_subcommand_from {all_subparsers}"''' for action_name, subparser in subparsers.choices.items() ) + ('\n# global flags',) + tuple( - '''complete -c borgmatic -a '%s' -d %s %s''' - % (' '.join(action.option_strings), shlex.quote(action.help), build_fish_flags(action)) + f'''complete -c borgmatic -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} {build_fish_flags(action)}''' for action in top_level_parser._actions ) + ('\n# subparser flags',) + tuple( - '''complete -c borgmatic -a '%s' -d %s -n "__fish_seen_subcommand_from %s" %s''' - % ( - ' '.join(action.option_strings), - shlex.quote(action.help), - action_name, - build_fish_flags(action), - ) + f'''complete -c borgmatic -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}" {build_fish_flags(action)}''' for action_name, subparser in subparsers.choices.items() for action in subparser._actions ) From 700f8e9d9c1ccd7e67a05a0d63647245a057272b Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 13:39:48 -0700 Subject: [PATCH 18/49] replace .format with fstring --- borgmatic/commands/completion.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 6a686d2bc..e206df73a 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -39,13 +39,11 @@ def bash_completion(): ' local this_script="$(cat "$BASH_SOURCE" 2> /dev/null)"', ' local installed_script="$(borgmatic --bash-completion 2> /dev/null)"', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ];' - ' then cat << EOF\n{}\nEOF'.format( - upgrade_message( + f''' then cat << EOF\n{upgrade_message( 'bash', 'sudo sh -c "borgmatic --bash-completion > $BASH_SOURCE"', '$BASH_SOURCE', - ) - ), + )}\nEOF''', ' fi', '}', 'complete_borgmatic() {', @@ -95,13 +93,11 @@ def fish_completion(): ' set this_script (cat $this_filename 2> /dev/null)', ' set installed_script (borgmatic --fish-completion 2> /dev/null)', ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ]', - ' echo "{}"'.format( - upgrade_message( + f''' echo "{upgrade_message( 'fish', 'borgmatic --fish-completion | sudo tee $this_filename', '$this_filename', - ) - ), + )}"''', ' end', 'end', '__borgmatic_check_version &', From f1fd2e88dd5d947341e37bbe2352ac1399ab5151 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 13:49:29 -0700 Subject: [PATCH 19/49] drop blank completion --- borgmatic/commands/completion.py | 1 + 1 file changed, 1 insertion(+) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index e206df73a..ec4d8d15d 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -111,6 +111,7 @@ def fish_completion(): + tuple( f'''complete -c borgmatic -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} {build_fish_flags(action)}''' for action in top_level_parser._actions + if len(action.option_strings) > 0 ) + ('\n# subparser flags',) + tuple( From 28efc8566075a53eff78783129d26111b0b804d8 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 18:11:13 -0700 Subject: [PATCH 20/49] rearrange to improve legability of the file --- borgmatic/commands/completion.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index ec4d8d15d..c8696e60b 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -102,9 +102,10 @@ def fish_completion(): 'end', '__borgmatic_check_version &', ) + + (f'''set --local subparser_condition "not __fish_seen_subcommand_from {all_subparsers}"''',) + ('\n# subparser completions',) + tuple( - f'''complete -c borgmatic -a '{action_name}' -d {shlex.quote(subparser.description)} -f -n "not __fish_seen_subcommand_from {all_subparsers}"''' + f'''complete -c borgmatic -n "$subparser_condition" -a '{action_name}' -d {shlex.quote(subparser.description)} -f''' for action_name, subparser in subparsers.choices.items() ) + ('\n# global flags',) From f12a10d888c2cdc5b5eb581dc40938605fb09fe2 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 19:50:49 -0700 Subject: [PATCH 21/49] start work on conditional file completion --- borgmatic/commands/completion.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index c8696e60b..5ae7a30ad 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -1,5 +1,6 @@ import shlex from argparse import Action +from textwrap import dedent from borgmatic.commands import arguments @@ -66,14 +67,20 @@ def bash_completion(): ) -def build_fish_flags(action: Action): +def conditionally_emit_file_completion(action: Action): ''' - Given an argparse.Action instance, return a string containing the fish flags for that action. + Given an argparse.Action instance, return a completion invocation that forces file completion + if the action takes a file argument and was the last action on the command line. + + Otherwise, return an empty string. ''' - if action.metavar and action.metavar == 'PATH' or action.metavar == 'FILENAME': - return '-r -F' - else: - return '-f' + if not action.metavar: + return '' + + args = ' '.join(action.option_strings) + + return dedent(f''' + complete -c borgmatic -a {args} -Fr -n "__borgmatic_last_arg {args}"''') def fish_completion(): @@ -105,18 +112,18 @@ def fish_completion(): + (f'''set --local subparser_condition "not __fish_seen_subcommand_from {all_subparsers}"''',) + ('\n# subparser completions',) + tuple( - f'''complete -c borgmatic -n "$subparser_condition" -a '{action_name}' -d {shlex.quote(subparser.description)} -f''' + f'''complete -c borgmatic -f -n "$subparser_condition" -a '{action_name}' -d {shlex.quote(subparser.description)}''' for action_name, subparser in subparsers.choices.items() ) + ('\n# global flags',) + tuple( - f'''complete -c borgmatic -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} {build_fish_flags(action)}''' + f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{conditionally_emit_file_completion(action)}''' for action in top_level_parser._actions if len(action.option_strings) > 0 ) + ('\n# subparser flags',) + tuple( - f'''complete -c borgmatic -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}" {build_fish_flags(action)}''' + f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}"{conditionally_emit_file_completion(action)}''' for action_name, subparser in subparsers.choices.items() for action in subparser._actions ) From 639e88262e3d0f7a27fdb7359bec44f9fa22d318 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 20:17:26 -0700 Subject: [PATCH 22/49] create working file completion --- borgmatic/commands/completion.py | 56 +++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 16 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 5ae7a30ad..9a21334d2 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -76,11 +76,17 @@ def conditionally_emit_file_completion(action: Action): ''' if not action.metavar: return '' - + args = ' '.join(action.option_strings) - - return dedent(f''' - complete -c borgmatic -a {args} -Fr -n "__borgmatic_last_arg {args}"''') + + return dedent( + f''' + complete -c borgmatic -a '{args}' -Fr -n "__borgmatic_last_arg {args}"''' + ) + + +def dedent_strip_as_tuple(string: str): + return (dedent(string).strip("\n"),) def fish_completion(): @@ -94,22 +100,40 @@ def fish_completion(): # Avert your eyes. return '\n'.join( - ( - 'function __borgmatic_check_version', - ' set this_filename (status current-filename)', - ' set this_script (cat $this_filename 2> /dev/null)', - ' set installed_script (borgmatic --fish-completion 2> /dev/null)', - ' if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ]', - f''' echo "{upgrade_message( + dedent_strip_as_tuple( + f''' + function __borgmatic_check_version + set this_filename (status current-filename) + set this_script (cat $this_filename 2> /dev/null) + set installed_script (borgmatic --fish-completion 2> /dev/null) + if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ] + echo "{upgrade_message( 'fish', 'borgmatic --fish-completion | sudo tee $this_filename', '$this_filename', - )}"''', - ' end', - 'end', - '__borgmatic_check_version &', + )}" + end + end + __borgmatic_check_version & + + function __borgmatic_last_arg --description 'Check if any of the given arguments are the last on the command line' + set -l all_args (commandline -poc) + # premature optimization to avoid iterating all args if there aren't enough + # to have a last arg beyond borgmatic + if [ (count $all_args) -lt 2 ] + return 1 + end + for arg in $argv + if [ "$arg" = "$all_args[-1]" ] + return 0 + end + end + return 1 + end + + set --local subparser_condition "not __fish_seen_subcommand_from {all_subparsers}" + ''' ) - + (f'''set --local subparser_condition "not __fish_seen_subcommand_from {all_subparsers}"''',) + ('\n# subparser completions',) + tuple( f'''complete -c borgmatic -f -n "$subparser_condition" -a '{action_name}' -d {shlex.quote(subparser.description)}''' From bbc3e9d7173784058646ee3c8325b976a2b04034 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 21:12:24 -0700 Subject: [PATCH 23/49] show possible choices --- borgmatic/commands/completion.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 9a21334d2..fb82e2a48 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -66,6 +66,15 @@ def bash_completion(): ) ) +file_metavars = ( + 'FILENAME', + 'PATH', +) + +file_destinations = ( + 'config_paths' +) + def conditionally_emit_file_completion(action: Action): ''' @@ -74,15 +83,22 @@ def conditionally_emit_file_completion(action: Action): Otherwise, return an empty string. ''' - if not action.metavar: - return '' args = ' '.join(action.option_strings) - return dedent( - f''' - complete -c borgmatic -a '{args}' -Fr -n "__borgmatic_last_arg {args}"''' - ) + if action.metavar in file_metavars or action.dest in file_destinations: + return dedent( + f''' + complete -c borgmatic -a '{args}' -Fr -n "__borgmatic_last_arg {args}"''' + ) + + if action.choices: + return dedent( + f''' + complete -c borgmatic -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_last_arg {args}"''' + ) + + return '' def dedent_strip_as_tuple(string: str): From 193731a017a87b267f126fe4fb02a0bc52eff36d Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 21:14:48 -0700 Subject: [PATCH 24/49] rename function --- borgmatic/commands/completion.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index fb82e2a48..be94dd0fa 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -66,20 +66,20 @@ def bash_completion(): ) ) + file_metavars = ( 'FILENAME', 'PATH', ) -file_destinations = ( - 'config_paths' -) +file_destinations = 'config_paths' -def conditionally_emit_file_completion(action: Action): +def conditionally_emit_arg_completion(action: Action): ''' - Given an argparse.Action instance, return a completion invocation that forces file completion - if the action takes a file argument and was the last action on the command line. + Given an argparse.Action instance, return a completion invocation + that forces file completion or options completion, if the action + takes such an argument and was the last action on the command line. Otherwise, return an empty string. ''' @@ -91,7 +91,7 @@ def conditionally_emit_file_completion(action: Action): f''' complete -c borgmatic -a '{args}' -Fr -n "__borgmatic_last_arg {args}"''' ) - + if action.choices: return dedent( f''' @@ -157,13 +157,13 @@ def fish_completion(): ) + ('\n# global flags',) + tuple( - f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{conditionally_emit_file_completion(action)}''' + f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{conditionally_emit_arg_completion(action)}''' for action in top_level_parser._actions if len(action.option_strings) > 0 ) + ('\n# subparser flags',) + tuple( - f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}"{conditionally_emit_file_completion(action)}''' + f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}"{conditionally_emit_arg_completion(action)}''' for action_name, subparser in subparsers.choices.items() for action in subparser._actions ) From d962376a9dd3b1c643bbe3c22de88d329eafe577 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 21:58:30 -0700 Subject: [PATCH 25/49] refactor to only show specific options if possible --- borgmatic/commands/completion.py | 42 ++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index be94dd0fa..0b1deb4f3 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -75,7 +75,11 @@ file_metavars = ( file_destinations = 'config_paths' -def conditionally_emit_arg_completion(action: Action): +def has_exact_options(action: Action): + return action.metavar in file_metavars or action.dest in file_destinations or action.choices + + +def exact_options_completion(action: Action): ''' Given an argparse.Action instance, return a completion invocation that forces file completion or options completion, if the action @@ -84,21 +88,20 @@ def conditionally_emit_arg_completion(action: Action): Otherwise, return an empty string. ''' + if not has_exact_options(action): + return '' + args = ' '.join(action.option_strings) if action.metavar in file_metavars or action.dest in file_destinations: - return dedent( - f''' - complete -c borgmatic -a '{args}' -Fr -n "__borgmatic_last_arg {args}"''' - ) + return f'''\ncomplete -c borgmatic -Fr -a '{args}' -n "__borgmatic_last_arg {args}"''' if action.choices: - return dedent( - f''' - complete -c borgmatic -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_last_arg {args}"''' - ) + return f'''\ncomplete -c borgmatic -f -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_last_arg {args}"''' - return '' + raise RuntimeError( + f'Unexpected action: {action} passes has_exact_options but has no choices produced' + ) def dedent_strip_as_tuple(string: str): @@ -114,6 +117,18 @@ def fish_completion(): all_subparsers = ' '.join(action for action in subparsers.choices.keys()) + exact_option_args = tuple( + ' '.join(action.option_strings) + for subparser in subparsers.choices.values() + for action in subparser._actions + if has_exact_options(action) + ) + tuple( + ' '.join(action.option_strings) + for action in top_level_parser._actions + if len(action.option_strings) > 0 + if has_exact_options(action) + ) + # Avert your eyes. return '\n'.join( dedent_strip_as_tuple( @@ -148,22 +163,23 @@ def fish_completion(): end set --local subparser_condition "not __fish_seen_subcommand_from {all_subparsers}" + set --local exact_option_condition "not __borgmatic_last_arg {' '.join(exact_option_args)}" ''' ) + ('\n# subparser completions',) + tuple( - f'''complete -c borgmatic -f -n "$subparser_condition" -a '{action_name}' -d {shlex.quote(subparser.description)}''' + f'''complete -c borgmatic -f -n "$subparser_condition" -n "$exact_option_condition" -a '{action_name}' -d {shlex.quote(subparser.description)}''' for action_name, subparser in subparsers.choices.items() ) + ('\n# global flags',) + tuple( - f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{conditionally_emit_arg_completion(action)}''' + f'''complete -c borgmatic -f -n "$exact_option_condition" -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{exact_options_completion(action)}''' for action in top_level_parser._actions if len(action.option_strings) > 0 ) + ('\n# subparser flags',) + tuple( - f'''complete -c borgmatic -f -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}"{conditionally_emit_arg_completion(action)}''' + f'''complete -c borgmatic -f -n "$exact_option_condition" -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}"{exact_options_completion(action)}''' for action_name, subparser in subparsers.choices.items() for action in subparser._actions ) From b4a38d8be9428a49eb3d47ef3fbda896d81eed95 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 23:06:11 -0700 Subject: [PATCH 26/49] fix flag showing up for paths --- borgmatic/commands/completion.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 0b1deb4f3..0f6c3c201 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -94,7 +94,7 @@ def exact_options_completion(action: Action): args = ' '.join(action.option_strings) if action.metavar in file_metavars or action.dest in file_destinations: - return f'''\ncomplete -c borgmatic -Fr -a '{args}' -n "__borgmatic_last_arg {args}"''' + return f'''\ncomplete -c borgmatic -Fr -n "__borgmatic_last_arg {args}"''' if action.choices: return f'''\ncomplete -c borgmatic -f -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_last_arg {args}"''' From 8f3039be2332c348810f330ede2466b529db0290 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 23:23:29 -0700 Subject: [PATCH 27/49] handle the expanding filters better --- borgmatic/commands/completion.py | 38 +++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 0f6c3c201..c22e50fd1 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -67,16 +67,35 @@ def bash_completion(): ) -file_metavars = ( - 'FILENAME', - 'PATH', -) +# fish section -file_destinations = 'config_paths' +def has_file_options(action: Action): + return action.metavar in ( + 'FILENAME', + 'PATH', + ) or action.dest in ('config_paths',) + + +def has_choice_options(action: Action): + return action.choices is not None + + +def has_required_param_options(action: Action): + return ( + action.nargs + in ( + "+", + "*", + ) + or '--archive' in action.option_strings + or action.metavar in ('PATTERN', 'KEYS', 'N') + ) def has_exact_options(action: Action): - return action.metavar in file_metavars or action.dest in file_destinations or action.choices + return ( + has_file_options(action) or has_choice_options(action) or has_required_param_options(action) + ) def exact_options_completion(action: Action): @@ -93,12 +112,15 @@ def exact_options_completion(action: Action): args = ' '.join(action.option_strings) - if action.metavar in file_metavars or action.dest in file_destinations: + if has_file_options(action): return f'''\ncomplete -c borgmatic -Fr -n "__borgmatic_last_arg {args}"''' - if action.choices: + if has_choice_options(action): return f'''\ncomplete -c borgmatic -f -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_last_arg {args}"''' + if has_required_param_options(action): + return f'''\ncomplete -c borgmatic -x -n "__borgmatic_last_arg {args}"''' + raise RuntimeError( f'Unexpected action: {action} passes has_exact_options but has no choices produced' ) From 3592ec3ddf44dd45f256d25b239bf0450a03bd99 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 23:32:09 -0700 Subject: [PATCH 28/49] dont show deprecated options --- borgmatic/commands/completion.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index c22e50fd1..eb45799fe 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -198,11 +198,13 @@ def fish_completion(): f'''complete -c borgmatic -f -n "$exact_option_condition" -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{exact_options_completion(action)}''' for action in top_level_parser._actions if len(action.option_strings) > 0 + if 'Deprecated' not in action.help ) + ('\n# subparser flags',) + tuple( f'''complete -c borgmatic -f -n "$exact_option_condition" -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)} -n "__fish_seen_subcommand_from {action_name}"{exact_options_completion(action)}''' for action_name, subparser in subparsers.choices.items() for action in subparser._actions + if 'Deprecated' not in action.help ) ) From 16ac4824a51965d55162044e43edcba8f01b6102 Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 23:42:04 -0700 Subject: [PATCH 29/49] handle typed without default params --- borgmatic/commands/completion.py | 1 + 1 file changed, 1 insertion(+) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index eb45799fe..d24c0430c 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -89,6 +89,7 @@ def has_required_param_options(action: Action): ) or '--archive' in action.option_strings or action.metavar in ('PATTERN', 'KEYS', 'N') + or (action.type is not None and action.default is None) ) From d59b9b817f3ebf8ab4f60b664c6fbbfbc49da9fd Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 23:44:54 -0700 Subject: [PATCH 30/49] support required actions --- borgmatic/commands/completion.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index d24c0430c..ee83c28ed 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -69,6 +69,7 @@ def bash_completion(): # fish section + def has_file_options(action: Action): return action.metavar in ( 'FILENAME', @@ -82,7 +83,8 @@ def has_choice_options(action: Action): def has_required_param_options(action: Action): return ( - action.nargs + action.required is True + or action.nargs in ( "+", "*", From b557d635fd18c6c952ff26432f396e9f6cfef37d Mon Sep 17 00:00:00 2001 From: Isaac Date: Thu, 4 May 2023 23:57:37 -0700 Subject: [PATCH 31/49] async validity check --- borgmatic/commands/completion.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index ee83c28ed..2dc63e670 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -159,18 +159,20 @@ def fish_completion(): dedent_strip_as_tuple( f''' function __borgmatic_check_version - set this_filename (status current-filename) - set this_script (cat $this_filename 2> /dev/null) - set installed_script (borgmatic --fish-completion 2> /dev/null) - if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ] - echo "{upgrade_message( - 'fish', - 'borgmatic --fish-completion | sudo tee $this_filename', - '$this_filename', - )}" - end + set -fx this_filename (status current-filename) + fish -c ' + set this_script (cat $this_filename 2> /dev/null) + set installed_script (borgmatic --fish-completion 2> /dev/null) + if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ] + echo "{upgrade_message( + 'fish', + 'borgmatic --fish-completion | sudo tee $this_filename', + '$this_filename', + )}" + end + ' & end - __borgmatic_check_version & + __borgmatic_check_version function __borgmatic_last_arg --description 'Check if any of the given arguments are the last on the command line' set -l all_args (commandline -poc) From 5a7a1747f29a16e8b3c2508ee0dcf2def5c58b85 Mon Sep 17 00:00:00 2001 From: Isaac Date: Fri, 5 May 2023 00:01:45 -0700 Subject: [PATCH 32/49] add safety check to avoid infinite cat hang --- borgmatic/commands/completion.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 2dc63e670..3ce5c03e1 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -161,14 +161,16 @@ def fish_completion(): function __borgmatic_check_version set -fx this_filename (status current-filename) fish -c ' - set this_script (cat $this_filename 2> /dev/null) - set installed_script (borgmatic --fish-completion 2> /dev/null) - if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ] - echo "{upgrade_message( - 'fish', - 'borgmatic --fish-completion | sudo tee $this_filename', - '$this_filename', - )}" + if test -f "$this_filename" + set this_script (cat $this_filename 2> /dev/null) + set installed_script (borgmatic --fish-completion 2> /dev/null) + if [ "$this_script" != "$installed_script" ] && [ "$installed_script" != "" ] + echo "{upgrade_message( + 'fish', + 'borgmatic --fish-completion | sudo tee $this_filename', + '$this_filename', + )}" + end end ' & end From 59a6ce1462d87dea5ebabbecf4bb4e18f3e80d44 Mon Sep 17 00:00:00 2001 From: Isaac Date: Fri, 5 May 2023 00:03:43 -0700 Subject: [PATCH 33/49] replace double quotes with single quotes --- borgmatic/commands/completion.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 3ce5c03e1..e99f39038 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -86,8 +86,8 @@ def has_required_param_options(action: Action): action.required is True or action.nargs in ( - "+", - "*", + '+', + '*', ) or '--archive' in action.option_strings or action.metavar in ('PATTERN', 'KEYS', 'N') @@ -130,7 +130,7 @@ def exact_options_completion(action: Action): def dedent_strip_as_tuple(string: str): - return (dedent(string).strip("\n"),) + return (dedent(string).strip('\n'),) def fish_completion(): From 469e0ccace89270ff3763614ad125ccacd956702 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 10:42:06 -0700 Subject: [PATCH 34/49] create doccomments, start writing unit tests --- borgmatic/commands/completion.py | 11 ++++++++--- tests/unit/commands/test_completions.py | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 tests/unit/commands/test_completions.py diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index e99f39038..e160a8fc9 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -71,6 +71,9 @@ def bash_completion(): def has_file_options(action: Action): + ''' + Given an argparse.Action instance, return True if it takes a file argument. + ''' return action.metavar in ( 'FILENAME', 'PATH', @@ -78,6 +81,9 @@ def has_file_options(action: Action): def has_choice_options(action: Action): + ''' + Given an argparse.Action instance, return True if it takes one of a predefined set of arguments. + ''' return action.choices is not None @@ -103,9 +109,8 @@ def has_exact_options(action: Action): def exact_options_completion(action: Action): ''' - Given an argparse.Action instance, return a completion invocation - that forces file completion or options completion, if the action - takes such an argument and was the last action on the command line. + Given an argparse.Action instance, return a completion invocation that forces file completion or options + completion, if the action takes such an argument and was the last action on the command line. Otherwise, return an empty string. ''' diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py new file mode 100644 index 000000000..4cc1f456e --- /dev/null +++ b/tests/unit/commands/test_completions.py @@ -0,0 +1,21 @@ +from argparse import Action + +import pytest + +from borgmatic.commands.completion import has_exact_options, has_file_options + +file_options_test_data = [ + (Action('--flag', 'flag'), False), + (Action('--flag', 'flag', metavar='FILENAME'), True), + (Action('--flag', 'flag', metavar='PATH'), True), + (Action('--flag', dest='config_paths'), True), + (Action('--flag', 'flag', metavar='OTHER'), False), +] + + +@pytest.mark.parametrize('action, expected', file_options_test_data) +def test_has_file_options_detects_file_options(action: Action, expected: bool): + assert has_file_options(action) == expected + # if has_file_options(action) was true, has_exact_options(action) should also be true + if expected: + assert has_exact_options(action) From 372622fbb107c4bbfee99c4e4f0d57e3e3a6e281 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 10:46:27 -0700 Subject: [PATCH 35/49] add more doccomments, drop a check --- borgmatic/commands/completion.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index e160a8fc9..702a09dd7 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -87,7 +87,13 @@ def has_choice_options(action: Action): return action.choices is not None -def has_required_param_options(action: Action): +def has_unknown_required_param_options(action: Action): + ''' + A catch-all for options that take a required parameter, but we don't know what the parameter is. + This should be used last. These are actions that take something like a glob, a list of numbers, or a string. + There is no way to know what the valid options are, but we need to prevent another argument from being shown, + and let the user know that they need to provide a parameter. + ''' return ( action.required is True or action.nargs @@ -95,7 +101,6 @@ def has_required_param_options(action: Action): '+', '*', ) - or '--archive' in action.option_strings or action.metavar in ('PATTERN', 'KEYS', 'N') or (action.type is not None and action.default is None) ) @@ -103,7 +108,9 @@ def has_required_param_options(action: Action): def has_exact_options(action: Action): return ( - has_file_options(action) or has_choice_options(action) or has_required_param_options(action) + has_file_options(action) + or has_choice_options(action) + or has_unknown_required_param_options(action) ) @@ -126,7 +133,7 @@ def exact_options_completion(action: Action): if has_choice_options(action): return f'''\ncomplete -c borgmatic -f -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_last_arg {args}"''' - if has_required_param_options(action): + if has_unknown_required_param_options(action): return f'''\ncomplete -c borgmatic -x -n "__borgmatic_last_arg {args}"''' raise RuntimeError( From e623f401b90031c7bcb948cad9f25574c614196b Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 10:56:54 -0700 Subject: [PATCH 36/49] write more unit tests --- tests/unit/commands/test_completions.py | 42 ++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 4cc1f456e..4de34fe70 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -2,7 +2,12 @@ from argparse import Action import pytest -from borgmatic.commands.completion import has_exact_options, has_file_options +from borgmatic.commands.completion import ( + has_choice_options, + has_exact_options, + has_file_options, + has_unknown_required_param_options, +) file_options_test_data = [ (Action('--flag', 'flag'), False), @@ -19,3 +24,38 @@ def test_has_file_options_detects_file_options(action: Action, expected: bool): # if has_file_options(action) was true, has_exact_options(action) should also be true if expected: assert has_exact_options(action) + + +choices_test_data = [ + (Action('--flag', 'flag'), False), + (Action('--flag', 'flag', choices=['a', 'b']), True), + (Action('--flag', 'flag', choices=None), False), +] + + +@pytest.mark.parametrize('action, expected', choices_test_data) +def test_has_choice_options_detects_choice_options(action: Action, expected: bool): + assert has_choice_options(action) == expected + # if has_choice_options(action) was true, has_exact_options(action) should also be true + if expected: + assert has_exact_options(action) + + +unknown_required_param_test_data = [ + (Action('--flag', 'flag'), False), + (Action('--flag', 'flag', required=True), True), + *((Action('--flag', 'flag', nargs=nargs), True) for nargs in ('+', '*')), + *((Action('--flag', 'flag', metavar=metavar), True) for metavar in ('PATTERN', 'KEYS', 'N')), + *((Action('--flag', 'flag', type=type, default=None), True) for type in (int, str)), + (Action('--flag', 'flag', type=int, default=1), False), +] + + +@pytest.mark.parametrize('action, expected', unknown_required_param_test_data) +def test_has_unknown_required_param_options_detects_unknown_required_param_options( + action: Action, expected: bool +): + assert has_unknown_required_param_options(action) == expected + # if has_unknown_required_param_options(action) was true, has_exact_options(action) should also be true + if expected: + assert has_exact_options(action) From 77dbb5c499d19ca40f98f68d8f91acb9c3939e8a Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:16:45 -0700 Subject: [PATCH 37/49] create way for test cases to be shared --- tests/unit/commands/test_completions.py | 129 +++++++++++++++++------- 1 file changed, 94 insertions(+), 35 deletions(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 4de34fe70..51ab29ac4 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -1,4 +1,6 @@ from argparse import Action +from collections import namedtuple +from typing import Tuple import pytest @@ -9,53 +11,110 @@ from borgmatic.commands.completion import ( has_unknown_required_param_options, ) -file_options_test_data = [ - (Action('--flag', 'flag'), False), - (Action('--flag', 'flag', metavar='FILENAME'), True), - (Action('--flag', 'flag', metavar='PATH'), True), - (Action('--flag', dest='config_paths'), True), - (Action('--flag', 'flag', metavar='OTHER'), False), +OptionType = namedtuple('OptionType', ['file', 'choice', 'unknown_required']) +TestCase = Tuple[Action, OptionType] + +test_data: list[TestCase] = [ + (Action('--flag', 'flag'), OptionType(file=False, choice=False, unknown_required=False)), + *( + ( + Action('--flag', 'flag', metavar=metavar), + OptionType(file=True, choice=False, unknown_required=False), + ) + for metavar in ('FILENAME', 'PATH') + ), + ( + Action('--flag', dest='config_paths'), + OptionType(file=True, choice=False, unknown_required=False), + ), + ( + Action('--flag', 'flag', metavar='OTHER'), + OptionType(file=False, choice=False, unknown_required=False), + ), + ( + Action('--flag', 'flag', choices=['a', 'b']), + OptionType(file=False, choice=True, unknown_required=False), + ), + ( + Action('--flag', 'flag', choices=['a', 'b'], type=str), + OptionType(file=False, choice=True, unknown_required=True), + ), + ( + Action('--flag', 'flag', choices=None), + OptionType(file=False, choice=False, unknown_required=False), + ), + ( + Action('--flag', 'flag', required=True), + OptionType(file=False, choice=False, unknown_required=True), + ), + *( + ( + Action('--flag', 'flag', nargs=nargs), + OptionType(file=False, choice=False, unknown_required=True), + ) + for nargs in ('+', '*') + ), + *( + ( + Action('--flag', 'flag', metavar=metavar), + OptionType(file=False, choice=False, unknown_required=True), + ) + for metavar in ('PATTERN', 'KEYS', 'N') + ), + *( + ( + Action('--flag', 'flag', type=type, default=None), + OptionType(file=False, choice=False, unknown_required=True), + ) + for type in (int, str) + ), + ( + Action('--flag', 'flag', type=int, default=1), + OptionType(file=False, choice=False, unknown_required=False), + ), + ( + Action('--flag', 'flag', type=str, required=True, metavar='PATH'), + OptionType(file=True, choice=False, unknown_required=True), + ), + ( + Action('--flag', 'flag', type=str, required=True, metavar='PATH', default='/dev/null'), + OptionType(file=True, choice=False, unknown_required=True), + ), + ( + Action('--flag', 'flag', type=str, required=False, metavar='PATH', default='/dev/null'), + OptionType(file=True, choice=False, unknown_required=False), + ), ] -@pytest.mark.parametrize('action, expected', file_options_test_data) -def test_has_file_options_detects_file_options(action: Action, expected: bool): - assert has_file_options(action) == expected +@pytest.mark.parametrize('action, option_type', test_data) +def test_has_file_options_detects_file_options(action: Action, option_type: OptionType): + assert has_file_options(action) == option_type.file # if has_file_options(action) was true, has_exact_options(action) should also be true - if expected: + if option_type.file: assert has_exact_options(action) -choices_test_data = [ - (Action('--flag', 'flag'), False), - (Action('--flag', 'flag', choices=['a', 'b']), True), - (Action('--flag', 'flag', choices=None), False), -] - - -@pytest.mark.parametrize('action, expected', choices_test_data) -def test_has_choice_options_detects_choice_options(action: Action, expected: bool): - assert has_choice_options(action) == expected +@pytest.mark.parametrize('action, option_type', test_data) +def test_has_choice_options_detects_choice_options(action: Action, option_type: OptionType): + assert has_choice_options(action) == option_type.choice # if has_choice_options(action) was true, has_exact_options(action) should also be true - if expected: + if option_type.choice: assert has_exact_options(action) -unknown_required_param_test_data = [ - (Action('--flag', 'flag'), False), - (Action('--flag', 'flag', required=True), True), - *((Action('--flag', 'flag', nargs=nargs), True) for nargs in ('+', '*')), - *((Action('--flag', 'flag', metavar=metavar), True) for metavar in ('PATTERN', 'KEYS', 'N')), - *((Action('--flag', 'flag', type=type, default=None), True) for type in (int, str)), - (Action('--flag', 'flag', type=int, default=1), False), -] - - -@pytest.mark.parametrize('action, expected', unknown_required_param_test_data) +@pytest.mark.parametrize('action, option_type', test_data) def test_has_unknown_required_param_options_detects_unknown_required_param_options( - action: Action, expected: bool + action: Action, option_type: OptionType ): - assert has_unknown_required_param_options(action) == expected + assert has_unknown_required_param_options(action) == option_type.unknown_required # if has_unknown_required_param_options(action) was true, has_exact_options(action) should also be true - if expected: + if option_type.unknown_required: assert has_exact_options(action) + + +@pytest.mark.parametrize('action, option_type', test_data) +def test_has_exact_options_detects_exact_options(action: Action, option_type: OptionType): + assert has_exact_options(action) == ( + option_type.file or option_type.choice or option_type.unknown_required + ) From aa564ac5fef2be1c13d1178c9b482f326c053baf Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:25:15 -0700 Subject: [PATCH 38/49] fix the error thrown, unit test for it, and add string explanations --- borgmatic/commands/completion.py | 2 +- tests/unit/commands/test_completions.py | 35 ++++++++++++++++--------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 702a09dd7..65ce415b7 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -136,7 +136,7 @@ def exact_options_completion(action: Action): if has_unknown_required_param_options(action): return f'''\ncomplete -c borgmatic -x -n "__borgmatic_last_arg {args}"''' - raise RuntimeError( + raise ValueError( f'Unexpected action: {action} passes has_exact_options but has no choices produced' ) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 51ab29ac4..736230967 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -5,6 +5,7 @@ from typing import Tuple import pytest from borgmatic.commands.completion import ( + exact_options_completion, has_choice_options, has_exact_options, has_file_options, @@ -89,32 +90,40 @@ test_data: list[TestCase] = [ @pytest.mark.parametrize('action, option_type', test_data) def test_has_file_options_detects_file_options(action: Action, option_type: OptionType): - assert has_file_options(action) == option_type.file - # if has_file_options(action) was true, has_exact_options(action) should also be true - if option_type.file: - assert has_exact_options(action) + assert ( + has_file_options(action) == option_type.file + ), f'Action: {action} should be file={option_type.file}' @pytest.mark.parametrize('action, option_type', test_data) def test_has_choice_options_detects_choice_options(action: Action, option_type: OptionType): - assert has_choice_options(action) == option_type.choice - # if has_choice_options(action) was true, has_exact_options(action) should also be true - if option_type.choice: - assert has_exact_options(action) + assert ( + has_choice_options(action) == option_type.choice + ), f'Action: {action} should be choice={option_type.choice}' @pytest.mark.parametrize('action, option_type', test_data) def test_has_unknown_required_param_options_detects_unknown_required_param_options( action: Action, option_type: OptionType ): - assert has_unknown_required_param_options(action) == option_type.unknown_required - # if has_unknown_required_param_options(action) was true, has_exact_options(action) should also be true - if option_type.unknown_required: - assert has_exact_options(action) + assert ( + has_unknown_required_param_options(action) == option_type.unknown_required + ), f'Action: {action} should be unknown_required={option_type.unknown_required}' @pytest.mark.parametrize('action, option_type', test_data) def test_has_exact_options_detects_exact_options(action: Action, option_type: OptionType): assert has_exact_options(action) == ( option_type.file or option_type.choice or option_type.unknown_required - ) + ), f'Action: {action} should have exact options given {option_type}' + + +@pytest.mark.parametrize('action, option_type', test_data) +def test_produce_exact_options_completion(action: Action, option_type: OptionType): + try: + completion = exact_options_completion(action) + assert ( + type(completion) == str + ), f'Completion should be a string, got {completion} of type {type(completion)}' + except ValueError as value_error: + assert False, f'exact_options_completion raised ValueError: {value_error}' From ccfdd6806f51fed91c6778eb3f66ab6313f4c7d8 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:29:14 -0700 Subject: [PATCH 39/49] test the value of completions --- tests/unit/commands/test_completions.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 736230967..74502775d 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -114,7 +114,7 @@ def test_has_unknown_required_param_options_detects_unknown_required_param_optio @pytest.mark.parametrize('action, option_type', test_data) def test_has_exact_options_detects_exact_options(action: Action, option_type: OptionType): assert has_exact_options(action) == ( - option_type.file or option_type.choice or option_type.unknown_required + True in option_type ), f'Action: {action} should have exact options given {option_type}' @@ -122,8 +122,12 @@ def test_has_exact_options_detects_exact_options(action: Action, option_type: Op def test_produce_exact_options_completion(action: Action, option_type: OptionType): try: completion = exact_options_completion(action) - assert ( - type(completion) == str - ), f'Completion should be a string, got {completion} of type {type(completion)}' + if True in option_type: + assert completion.startswith( + '\ncomplete -c borgmatic' + ), f'Completion should start with "complete -c borgmatic", got {completion}' + else: + assert completion == '', f'Completion should be empty, got {completion}' + except ValueError as value_error: assert False, f'exact_options_completion raised ValueError: {value_error}' From d7320599795bac4f6a2605fa7f630a5282e6f50d Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:32:10 -0700 Subject: [PATCH 40/49] fix rotted comments --- borgmatic/commands/completion.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 65ce415b7..e39f742de 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -116,8 +116,9 @@ def has_exact_options(action: Action): def exact_options_completion(action: Action): ''' - Given an argparse.Action instance, return a completion invocation that forces file completion or options - completion, if the action takes such an argument and was the last action on the command line. + Given an argparse.Action instance, return a completion invocation that forces file completions, options completion, + or just that some value follow the action, if the action takes such an argument and was the last action on the + command line prior to the cursor. Otherwise, return an empty string. ''' @@ -188,7 +189,7 @@ def fish_completion(): end __borgmatic_check_version - function __borgmatic_last_arg --description 'Check if any of the given arguments are the last on the command line' + function __borgmatic_last_arg --description 'Check if any of the given arguments are the last on the command line before the cursor' set -l all_args (commandline -poc) # premature optimization to avoid iterating all args if there aren't enough # to have a last arg beyond borgmatic From a047f856a13c7cce4f7463ad2d9fa0a01c695d76 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:37:38 -0700 Subject: [PATCH 41/49] tweak docstring, add comment --- borgmatic/commands/completion.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index e39f742de..555ffcf9e 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -91,8 +91,8 @@ def has_unknown_required_param_options(action: Action): ''' A catch-all for options that take a required parameter, but we don't know what the parameter is. This should be used last. These are actions that take something like a glob, a list of numbers, or a string. - There is no way to know what the valid options are, but we need to prevent another argument from being shown, - and let the user know that they need to provide a parameter. + + Actions that match this pattern should not show the normal arguments, because those are unlikely to be valid. ''' return ( action.required is True @@ -215,6 +215,7 @@ def fish_completion(): ) + ('\n# global flags',) + tuple( + # -n is checked in order, so put faster / more likely to be true checks first f'''complete -c borgmatic -f -n "$exact_option_condition" -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{exact_options_completion(action)}''' for action in top_level_parser._actions if len(action.option_strings) > 0 From c8f4344f8968aa9c61a8c7f047b091f6f9854868 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:39:02 -0700 Subject: [PATCH 42/49] add more justification to checks --- borgmatic/commands/completion.py | 1 + 1 file changed, 1 insertion(+) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index 555ffcf9e..affe5f971 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -218,6 +218,7 @@ def fish_completion(): # -n is checked in order, so put faster / more likely to be true checks first f'''complete -c borgmatic -f -n "$exact_option_condition" -a '{' '.join(action.option_strings)}' -d {shlex.quote(action.help)}{exact_options_completion(action)}''' for action in top_level_parser._actions + # ignore the noargs action, as this is an impossible completion for fish if len(action.option_strings) > 0 if 'Deprecated' not in action.help ) From efb81fc2c1ea61fdabf77ecbba27214a359a0ec9 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:42:32 -0700 Subject: [PATCH 43/49] rename last arg helper function to current arg for clarity --- borgmatic/commands/completion.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index affe5f971..d0ce7452f 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -129,13 +129,13 @@ def exact_options_completion(action: Action): args = ' '.join(action.option_strings) if has_file_options(action): - return f'''\ncomplete -c borgmatic -Fr -n "__borgmatic_last_arg {args}"''' + return f'''\ncomplete -c borgmatic -Fr -n "__borgmatic_current_arg {args}"''' if has_choice_options(action): - return f'''\ncomplete -c borgmatic -f -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_last_arg {args}"''' + return f'''\ncomplete -c borgmatic -f -a '{' '.join(map(str, action.choices))}' -n "__borgmatic_current_arg {args}"''' if has_unknown_required_param_options(action): - return f'''\ncomplete -c borgmatic -x -n "__borgmatic_last_arg {args}"''' + return f'''\ncomplete -c borgmatic -x -n "__borgmatic_current_arg {args}"''' raise ValueError( f'Unexpected action: {action} passes has_exact_options but has no choices produced' @@ -189,7 +189,7 @@ def fish_completion(): end __borgmatic_check_version - function __borgmatic_last_arg --description 'Check if any of the given arguments are the last on the command line before the cursor' + function __borgmatic_current_arg --description 'Check if any of the given arguments are the last on the command line before the cursor' set -l all_args (commandline -poc) # premature optimization to avoid iterating all args if there aren't enough # to have a last arg beyond borgmatic @@ -205,7 +205,7 @@ def fish_completion(): end set --local subparser_condition "not __fish_seen_subcommand_from {all_subparsers}" - set --local exact_option_condition "not __borgmatic_last_arg {' '.join(exact_option_args)}" + set --local exact_option_condition "not __borgmatic_current_arg {' '.join(exact_option_args)}" ''' ) + ('\n# subparser completions',) From 43c532bc577eb7169c5700fb42d74b805025ae66 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 11:51:35 -0700 Subject: [PATCH 44/49] add test for dedent strip --- borgmatic/commands/completion.py | 4 ++++ tests/unit/commands/test_completions.py | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/borgmatic/commands/completion.py b/borgmatic/commands/completion.py index d0ce7452f..4b2f17f38 100644 --- a/borgmatic/commands/completion.py +++ b/borgmatic/commands/completion.py @@ -143,6 +143,10 @@ def exact_options_completion(action: Action): def dedent_strip_as_tuple(string: str): + ''' + Dedent a string, then strip it to avoid requiring your first line to have content, then return a tuple of the string. + Makes it easier to write multiline strings for completions when you join them with a tuple. + ''' return (dedent(string).strip('\n'),) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 74502775d..69110af69 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -5,6 +5,7 @@ from typing import Tuple import pytest from borgmatic.commands.completion import ( + dedent_strip_as_tuple, exact_options_completion, has_choice_options, has_exact_options, @@ -131,3 +132,12 @@ def test_produce_exact_options_completion(action: Action, option_type: OptionTyp except ValueError as value_error: assert False, f'exact_options_completion raised ValueError: {value_error}' + + +def test_dedent_strip_as_tuple(): + dedent_strip_as_tuple( + ''' + a + b + ''' + ) From 0657106893b729a35227a8be7198b90982b214d9 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 15:46:15 -0700 Subject: [PATCH 45/49] clarify dedent test name --- tests/unit/commands/test_completions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 69110af69..b6d3103fb 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -134,7 +134,7 @@ def test_produce_exact_options_completion(action: Action, option_type: OptionTyp assert False, f'exact_options_completion raised ValueError: {value_error}' -def test_dedent_strip_as_tuple(): +def test_dedent_strip_as_tuple_does_not_raise(): dedent_strip_as_tuple( ''' a From 453b78c852fb36186dc3a6cc0a3f00681e4757ee Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 15:49:07 -0700 Subject: [PATCH 46/49] drop messages --- tests/unit/commands/test_completions.py | 32 +++++++------------------ 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index b6d3103fb..ba5622097 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -91,47 +91,33 @@ test_data: list[TestCase] = [ @pytest.mark.parametrize('action, option_type', test_data) def test_has_file_options_detects_file_options(action: Action, option_type: OptionType): - assert ( - has_file_options(action) == option_type.file - ), f'Action: {action} should be file={option_type.file}' + assert has_file_options(action) == option_type.file @pytest.mark.parametrize('action, option_type', test_data) def test_has_choice_options_detects_choice_options(action: Action, option_type: OptionType): - assert ( - has_choice_options(action) == option_type.choice - ), f'Action: {action} should be choice={option_type.choice}' + assert has_choice_options(action) == option_type.choice @pytest.mark.parametrize('action, option_type', test_data) def test_has_unknown_required_param_options_detects_unknown_required_param_options( action: Action, option_type: OptionType ): - assert ( - has_unknown_required_param_options(action) == option_type.unknown_required - ), f'Action: {action} should be unknown_required={option_type.unknown_required}' + assert has_unknown_required_param_options(action) == option_type.unknown_required @pytest.mark.parametrize('action, option_type', test_data) def test_has_exact_options_detects_exact_options(action: Action, option_type: OptionType): - assert has_exact_options(action) == ( - True in option_type - ), f'Action: {action} should have exact options given {option_type}' + assert has_exact_options(action) == (True in option_type) @pytest.mark.parametrize('action, option_type', test_data) def test_produce_exact_options_completion(action: Action, option_type: OptionType): - try: - completion = exact_options_completion(action) - if True in option_type: - assert completion.startswith( - '\ncomplete -c borgmatic' - ), f'Completion should start with "complete -c borgmatic", got {completion}' - else: - assert completion == '', f'Completion should be empty, got {completion}' - - except ValueError as value_error: - assert False, f'exact_options_completion raised ValueError: {value_error}' + completion = exact_options_completion(action) + if True in option_type: + assert completion.startswith('\ncomplete -c borgmatic') + else: + assert completion == '' def test_dedent_strip_as_tuple_does_not_raise(): From aa770b98f90652a3579cf4b21ae1a5b370a3c4b2 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 15:50:37 -0700 Subject: [PATCH 47/49] follow unit test module convention --- tests/unit/commands/test_completions.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index ba5622097..64ea3c7e0 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -4,14 +4,7 @@ from typing import Tuple import pytest -from borgmatic.commands.completion import ( - dedent_strip_as_tuple, - exact_options_completion, - has_choice_options, - has_exact_options, - has_file_options, - has_unknown_required_param_options, -) +from borgmatic.commands import completion as module OptionType = namedtuple('OptionType', ['file', 'choice', 'unknown_required']) TestCase = Tuple[Action, OptionType] @@ -91,29 +84,29 @@ test_data: list[TestCase] = [ @pytest.mark.parametrize('action, option_type', test_data) def test_has_file_options_detects_file_options(action: Action, option_type: OptionType): - assert has_file_options(action) == option_type.file + assert module.has_file_options(action) == option_type.file @pytest.mark.parametrize('action, option_type', test_data) def test_has_choice_options_detects_choice_options(action: Action, option_type: OptionType): - assert has_choice_options(action) == option_type.choice + assert module.has_choice_options(action) == option_type.choice @pytest.mark.parametrize('action, option_type', test_data) def test_has_unknown_required_param_options_detects_unknown_required_param_options( action: Action, option_type: OptionType ): - assert has_unknown_required_param_options(action) == option_type.unknown_required + assert module.has_unknown_required_param_options(action) == option_type.unknown_required @pytest.mark.parametrize('action, option_type', test_data) def test_has_exact_options_detects_exact_options(action: Action, option_type: OptionType): - assert has_exact_options(action) == (True in option_type) + assert module.has_exact_options(action) == (True in option_type) @pytest.mark.parametrize('action, option_type', test_data) def test_produce_exact_options_completion(action: Action, option_type: OptionType): - completion = exact_options_completion(action) + completion = module.exact_options_completion(action) if True in option_type: assert completion.startswith('\ncomplete -c borgmatic') else: @@ -121,7 +114,7 @@ def test_produce_exact_options_completion(action: Action, option_type: OptionTyp def test_dedent_strip_as_tuple_does_not_raise(): - dedent_strip_as_tuple( + module.dedent_strip_as_tuple( ''' a b From 614c1bf2e41ee189188a5a8c9ec85079a2d98989 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 15:52:42 -0700 Subject: [PATCH 48/49] rename test to make function under test clearer --- tests/unit/commands/test_completions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 64ea3c7e0..878e82044 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -105,7 +105,7 @@ def test_has_exact_options_detects_exact_options(action: Action, option_type: Op @pytest.mark.parametrize('action, option_type', test_data) -def test_produce_exact_options_completion(action: Action, option_type: OptionType): +def test_exact_options_completion_produces_reasonable_completions(action: Action, option_type: OptionType): completion = module.exact_options_completion(action) if True in option_type: assert completion.startswith('\ncomplete -c borgmatic') From 66964f613c1c8cd6e7fd3dece33cde430c8ed6b6 Mon Sep 17 00:00:00 2001 From: Isaac Date: Sat, 6 May 2023 15:56:50 -0700 Subject: [PATCH 49/49] formatting! --- tests/unit/commands/test_completions.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit/commands/test_completions.py b/tests/unit/commands/test_completions.py index 878e82044..acb01f6a5 100644 --- a/tests/unit/commands/test_completions.py +++ b/tests/unit/commands/test_completions.py @@ -105,7 +105,9 @@ def test_has_exact_options_detects_exact_options(action: Action, option_type: Op @pytest.mark.parametrize('action, option_type', test_data) -def test_exact_options_completion_produces_reasonable_completions(action: Action, option_type: OptionType): +def test_exact_options_completion_produces_reasonable_completions( + action: Action, option_type: OptionType +): completion = module.exact_options_completion(action) if True in option_type: assert completion.startswith('\ncomplete -c borgmatic')