mount/umount: Make MOUNTPOINT a positional aurgument instead of using --mount-point #348
Labels
No Label
bug
data loss
design finalized
good first issue
new feature area
question / support
security
waiting for response
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: borgmatic-collective/borgmatic#348
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?
What I'm trying to do and why
I just started using borgmatic (it's fantastic!) so I may be missing something here, but isn't a mount point always required in
borgmatic mount/umount
? If that's the case, I would suggest dropping the--mount-point
flag to simplify the syntax and make it more similar toborg mount/umount
(and also the traditionalmount/umount
commands).For example, instead of
borgmatic mount --archive some-archive --mount-point /mnt/backups
it would become
borgmatic mount --archive some-archive /mnt/backups
Other notes / implementation ideas
I would also suggest dropping the
--archive
flag, e.g.borgmatic mount some-archive /mnt/backups
To mount all archives or the latest archive, there could be e.g. a
--all
and a--latest
flag. Mounting all archives if--archive
is omitted doesn't sound like a very good default behavior to me.If this sounds good, I could post it under a separate issue. Just let me know.
Environment
borgmatic version: 1.5.9
The current set of
borgmatic mount
flags is actually a deliberate design decision. When I'm using commands likeborg
directly where required arguments are just positional, I often have a hard time remembering the order of the arguments.But with commands like
borgmatic mount
as currently implemented, I don't have to remember anything other than the names of flags, and I can specify them in any order I like. And required arguments don't have to come before optional arguments; they can be in the order that makes sense as I'm typing them.I realize this flies a little in the face the usual Unix convention. I also realize that this often requires more typing! But I feel like the trade-offs are worth it with a program like borgmatic. You'll notice similiar convention in all the other borgmatic sub-commands as well.
I will say that if you know of a way in Python (
argparse
) to make a sub-command accept a flag either as positional or as a--long-flag
, I'd be willing to make that a supported thing.Thanks for the explanation, that makes sense. I usually also prefer keyword arguments when in comes to e.g. Python functions. In this case, however, I guess I was expecting the
borgmatic mount/umount
commands to behave similarly to theirborg
/Unixmount/umount
counterparts.Here's a quick proof-of-concept I cooked up:
Some examples:
Not sure what would make most sense in the last case, should the positional argument be interpreted as
mount-point
sincearchive
has been given, or should it interpret it asarchive
(current situation) since it's the first positional argument?If you think this feature is worth implementing I could try to put together a pull request.
Hmm. Good question. I think it's probably okay to leave the current behavior (the multiple values error). When you mix positional and non-positional arguments like that, the right way to intrepret that is kind of undefined. The user can always switch to either 100% positional or 100% flags to disambiguate.
I think it's a question of whether you think it's worth implementing. :) I'd certainly entertain a pull request though if you're up to it. Based on your example, it does look like argparse could be made to support both positional and flag arguments.
One suggestion I have though is to make a canonical destination for each logical option (e.g.
args.archive
) and copy values from other sources (likeargs.archive_flag
) into it. That way, any downstream code that needs to access the archive argument doesn't need to be conditional, and can just consume from a single place (e.g.args.archive
).I'll take a look at the code and give it a try. :)
Awesome. :)
Closing due to inactivity. However, please feel free to send a pull request if you like!