--confirm

Suggestions and discussion of future versions
Post Reply
Puma
New User
New User
Posts: 6
Joined: Mon Nov 26, 2018 6:36 pm

--confirm

Post by Puma »

So issuing this command
$ deluge-console rm --confirm '*'
Witnessed behavior: everything gets removed.
Expected bahavior: List the matching torrents without removing.
I'm on

Code: Select all

Debian bullseye
deluge-console 2.0.3
libtorrent: 1.2.9.0
Python: 3.9.2
OS: Linux 5.10.0-8-amd64
PS. Also maybe add a --state option for the rm command as an additional way of filtering what gets removed.
mhertz
Moderator
Moderator
Posts: 2182
Joined: Wed Jan 22, 2014 5:05 am
Location: Denmark

Re: --confirm

Post by mhertz »

First I was about to post that you seemed little confused(respectfully of-course), but see you're actually right :) The text needs be fixed up I see, for the argument help text of rm/del command.

Code: Select all

martin@arch ~ % deluge-console help rm
usage: rm/del [-h] [--remove_data] [-c]  <torrent-id> [<torrent-id> ...]

Remove a torrent

positional arguments:
  
  <torrent-id>   One or more torrent ids

optional arguments:
  -h, --help     show this help message and exit
  --remove_data  Also removes the torrent data
  -c, --confirm  List the matching torrents without removing.
So disregard that text, and instead run without '--confirm' to get matches listed without removal, and add '--confirm' if specifically wanting to confirm removal of of said items.

'--state' addition sounds like a good idea, agreed. Btw, the '-s/--state' arg of 'info' command currently is broken and needs a fix previously posted here, curtesy of jbrid, which I made ticket for also - change line 194 of '/usr/lib/python*/*/deluge/ui/console/cmdline/commands/info.py' from 'status_dict.state = options.state' into 'status_dict = {'state': options.state}'.

For others reading this, avoid the short form of '--confirm' as '-c' overlaps with short form of '--config' by mistake and latter taking precedense, hence displaying "Password does not match" error when used(as profile-dir path overridden) - pending PR already submitted by Lucas-C.
Puma
New User
New User
Posts: 6
Joined: Mon Nov 26, 2018 6:36 pm

Re: --confirm

Post by Puma »

I'm little bit surprised that you were able to unconfuse yourself without any additional help (respectfully of course) considering that you are a python programmer. And from an UI design perspective the --no-act argument makes more sense than an --act argument. If the default behavior is to list, then it is a list command instead of a remove command. Acting by default and accepting a --no-act argument would also be in line with just about every other CLI program ever made. If you want a reasonable and safe default behavior, then it should be to prompt a [y/n] confirmation with every matching torrent. And you could then bypass the prompt by issuing a --yes --batch or a --force.

So instead of fixing the help text, maybe fix behavior instead. I see that you have already acknowledged the unintuitiveness of the current behavior by making the rm command print out an instruction prompting the user to use the --confirm argument if one is omitted. Printing out the instruction is essentially saying "Gee, we know that there is a good chance that what just happened isn't what you were expecting to happen when you issued the command. So here is what you need to do to get it to do what you were expecting." And if you need to say that, then there might be something wrong with your UI design.
mhertz
Moderator
Moderator
Posts: 2182
Joined: Wed Jan 22, 2014 5:05 am
Location: Denmark

Re: --confirm

Post by mhertz »

I'm little bit surprised that you were able to unconfuse yourself without any additional help (respectfully of course) considering that you are a python programmer.
Sorry don't really understand your comment here honestly. I feel it's a jab back of sorts, since guessing you took offence to my previous comment(which 100% wasen't ment in such way, but in hindsight probably shouldn't have posted like that, sorry about that), which is fine, not thinned skinned ussually(Edit: Get it know, so tongue in check jab back, not that one ever was given precedingly, atleast not in intent, but whatever and admittedly not best wording in hindsight, and hinting at my under-superiority at being supposedly a python programmer - pretty sure got that right now, don't know why was so hard to deduct for me, so probably correct after all lol :) ) , and just wanted to explain my previous comment was about you using a '--confirm' switch and expecting it to not act as a '--confirm' switch, e.g. kinda like a '-y' switch of apt-get etc. I thought you where referring to the help text and hence made the post like that.

Hmm, on second thought, while writing this, also taking into consideration your later comments, then I see it could actually be understood as a switch for wanting to get displayed confirmation additionally, but nonetheless despite me thinking the word "confirm" much more likely rings bells of beeing same as a '-y' switch, then as said now better understand the argument e.g my used distro's package manager has a '--noconfirm' switch doing same as deluge-console's '--confirm' switch :)
And from an UI design perspective the --no-act argument makes more sense than an --act argument. If the default behavior is to list, then it is a list command instead of a remove command. Acting by default and accepting a --no-act argument would also be in line with just about every other CLI program ever made. If you want a reasonable and safe default behavior, then it should be to prompt a [y/n] confirmation with every matching torrent. And you could then bypass the prompt by issuing a --yes --batch or a --force.
I agree in most your points, would be nice to have it output (y/n) and a '-y' switch etc. Don't feel "no-act" makes more sence imho, neither sure that majority of CLI apps that accepts confirmation defaults to "act", but could be wrong. Interesting point about 'rm' by default is a list command, never thought about that, but true :)
So instead of fixing the help text, maybe fix behavior instead. I see that you have already acknowledged the unintuitiveness of the current behavior by making the rm command print out an instruction prompting the user to use the --confirm argument if one is omitted. Printing out the instruction is essentially saying "Gee, we know that there is a good chance that what just happened isn't what you were expecting to happen when you issued the command. So here is what you need to do to get it to do what you were expecting." And if you need to say that, then there might be something wrong with your UI design.
Personally I don't care enough to make PR for this - i'm not a deluge dev btw, just fellow forum member, and not good programmer neither :)
Puma
New User
New User
Posts: 6
Joined: Mon Nov 26, 2018 6:36 pm

Re: --confirm

Post by Puma »

mhertz wrote: Wed Sep 15, 2021 3:41 pmDon't feel "no-act" makes more sence imho, neither sure that majority of CLI apps that accepts confirmation defaults to "act", but could be wrong.
There are essentially four "act" positions that a removing program can default to: "yes", "no", "ask-yes" and "ask-no". I don't think I have ever seen a program that can even be made "ask-yes", so there are really only three sane options.
$ rm -- defaults to "yes" but can be made made "ask-no" with the -i switch and back to "yes" with the -f switch
$ trash -- defaults to "yes" with no option to default to anything else
$ rsync -delete -- defaults to "yes" but can be made "no" with the -n switch
$ mv and cp -- can be used to "remove" files by overwriting them, in which case both default to "yes" but can be made "ask-no" with the -i switch and "no" with the -n switch
$ dd -- same as above, defaults to "yes" with no option to default to anything else
$ atrm -- defaults to "yes" with no option
$ lprm -- defaults to "yes" with no option
$ crontab -r -- defaults to "yes" with no option
$ 7z d -- defaults to "yes"
$ tar --delete -- defaults to "yes"

Those are the only command line programs, that remove stuff, I could think of off the top of my head. I'm sure there are other ones. And I'm sure most of them default to "yes" acting, not because it objectively makes more sense, but because it has become an established convention that users are expecting.

By requiring the switch for the command to work at all, rm --confirm has become the de-facto remove command. Which in my not-at-all-humble opinion is needlessly long and cumbersome CLI command. This is objectively bad UI design.
mhertz
Moderator
Moderator
Posts: 2182
Joined: Wed Jan 22, 2014 5:05 am
Location: Denmark

Re: --confirm

Post by mhertz »

Thank you for explaining, and examples, I gather you're probably right then, sorry for posting wrong info. The deluge devs usually respond most to tickets submitted, so if interested then probably will get more attention there, or could e.g link this thread with your suggestions to the pending PR by Lucas-C, not that his PR is about this, but still related to the '--confirm' command nonetheless(and 'rm' text), or could submit one yourself of-course.

Thanks for interessting discussion :)
Cas
Top Bloke
Top Bloke
Posts: 3679
Joined: Mon Dec 07, 2009 6:04 am
Location: Scotland

Re: --confirm

Post by Cas »

I agree there is some confusion here and improvement could be made. However we have to weigh up the potential for users to make mistakes with no way to undo especially with wildcard option. In fact with the GTK UI there was a discussion around user's accidentally deleting torrent data with torrent file and changes were made to prevent that.

From https://clig.dev/#arguments-and-flags I can see some possible ways forward:
wrote:Confirm before doing anything dangerous. A common convention is to prompt for the user to type y or yes if running interactively, or requiring them to pass -f or --force otherwise.
So perhaps use --force instead of --confirm, I don't think it is feasible to change the way rm works now. However perhaps an option could be added that could default to always delete.
Post Reply