Use sub-commands instead of options for actions #195
Labels
No Label
bug
data loss
design finalized
good first issue
new feature area
question / support
security
waiting for response
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: borgmatic-collective/borgmatic#195
Loading…
Reference in New Issue
No description provided.
Delete Branch "%!s(<nil>)"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
borgmatic uses options that are treated special for actions like create, list, etc.
This is generally unintuitive and even more so, as borg itself properly uses sub-commands (like many other tools like git do as well).
Furthermore, e.g.
--list
and--info
are valid options to borg'screate
sub-command, adding extra confusion and making it akward to support them as options to borgmatic's create action.Thus it would be more consistent (and two less characters to type 😉) if borgmatic used subcommands and e.g.
borgmatic list
did what one was meaning.I often forget to write the
--
in front ofborgmatic create
. Having Borgmatic accept sub-command would make its usage closer to Borg's usage.With Borg:
borg init
borg create
borg check
borg list
Today with Borgmatic:
borgmatic --init
borgmatic --create
borgmatic --check
borgmatic --list
The main reason borgmatic doesn't use "true" sub-commands is because:
--prune
,--create
, and--check
.For both of these, as far as I know, there's not a way with Python's
argparse
for a user to specify multiple sub-commands in a single invocation. And conceptually, it makes these actions much less like traditional sub-commands and more like.. just flags. While Borg has the benefit of exposing single commands independently, borgmatic is a higher-level wrapper that bundles up multiple commands together.Anyway, having said that, I'm open to brainstorming ways to change this existing behavior to get more towards the sub-command world.. Because I agree that it would be conceptually simpler if it's indeed possible. (I don't really like having to handle all of the edge cases of specifying multiple different actions together on a single invocation.)
As for @thomasleveil's idea of adding an alias of
init
for--init
,create
for--create
, that would probably be possible without any major changes.Thoughts on any of the above?
Personally, I'd work around the
argparse
limitation 1 by making abackup
sub-command that invokes the 3 others.But then, I'm biased anyway, IMO
--prune
(quite obviously with append-only repositories 😉) and--check
are better to be scheduled independently anyway.As for the aliases, that still doesn't solve any eventual support of options of the same name, ie.
borgmatic create --list
would do something quite different thenborg create --list
and you have to do away with the--list
action to allow for a--list
option to be added, for example.Spontaneous, slightly hackish idea: Do a
argparse
parser with the common options and doadd_argument('subcmds', metavar='COMMAND', nargs='+')
then you split thesubcmds
whenever there's an item not starting with a dash, ie.['create', '--list', '--stats', 'check', 'list']
becomes['create', '--list', '--stats'], ['check'], ['list']
and you then invoke the according separateargparse
parsers (that otherwise would be sub-parsers) on each of the split lists.You'd loose overall
--help
.I don't think we actually need to support Borg's
create --list
directly, because borgmatic already rolls that up into the concept of a verbosity level. This is from borgmatic'screate_archive()
code:And so I'm suggesting that
--list
doesn't need to be exposed directly to the user on the--create
orcreate
command-line.Anyway, I think your general idea of splitting arguments and passing them to separate parsers is interesting. I'll have to play around with that. Or maybe without needing to support
create --list
, the alias idea would be good enough?create --list
was just an example to make clear how to split and how having actions as positional arguments without dashes is unambiguous even if there is actions with the same names as options.I don't see the aliases working. If you let argparse do it (be it it accepting longoption aliases without dashes, or you preprocessing
argv
and substituting anyaction
with--action
), it'll loose order and thus which option was meant to apply to which action and option verification (which the argparse sub-parsers would handle) will become way more complex, ie. refusing--create --foo -bar
if--foo
and--bar
can not be combined within one operation (action), while allowing--create --foo --check --bar
.Yeah, for the alias thing, I wasn't thinking of it as a full substitute for sub-commands, and more of just an aesthetic change. They'd still require all of the same option parsing code that's there now, so they wouldn't save us from that. Anyway, I'll have a look at the separate parser idea.
I'm currently going with an approach with sub-commands where each sub-command's parser uses argparse's parse_known_args() on the full set of command-line arguments. This means that each sub-parser gets a whack at the full piñata of arguments. This isn't exactly what you asked for, so here are some pros and cons:
Pros:
borgmatic create
.borgmatic create --progress prune
.borgmatic create --progress
,borgmatic --progress create
.borgmatic --verbosity 1 create
,borgmatic create --verbosity 1
.borgmatic --create
.--help
since things likeborg create --help
can show create-specific help.Cons:
--progress
is anywhere on the command-line, all sub-commands that accept--progress
will consume it. I actually think this might be less confusing, but it's different than your ask above.--list
only getting passed to a single sub-command without triggering aborgmatic list
action. (Sub-commands with the same name as actions.) I think this is a small price to pay though.This work has landed in master, and will be part of the next release. Feel free to play around with it and provide any feedback.
Well, I decided to go ahead and release this. Mostly because docs now auto-publish from master, and I didn't want the online docs ahead of the released code! Anyway, you can grab borgmatic 1.3.9 and try this out.