CLI broken on 2.1 rc4

[mapreri]

[ Forwarded from https://bugs.debian.org/933909 ]

Previously, I would call subdownloader as
subdownloader -c --rename-subs -l en -V .
, and it would automatically download english subs for all movies in the current directory. :)

Now, it just dumps me at a poorly documented interactive prompt:

$ subdownloader -c --rename-video -l en -V .
SubDownloader 2.1.0rc4
Type "help" for more information.
>>>

I managed to use "filescan" to get it to scan the videos in the current directory, and "login" and "vidsearch" to possibly search for something.
I'm unable to figure out how to download subtitles, because there are "Number of subs to download: 0" according to "viddownload" , and "vidselect" just complains about "Value out of range." for every index I can think of.
Log attached

Version Info

• SubDownloader Version: 2.1.0 rc4
• Python Version: 3.7
• Operating System: Debian

Additional info

P.S.: Changing the option which names the newly downloaded subs according to the video filename from "--rename-subs" to
"--rename-video" seems really unfortunate.

[madebr]

Hey @mapreri ,
Thanks for the detailed report!

The goal of the console is to allow the same actions as the gui.
I agree it's underdocumented but its names are subject to change.
I just pushed some new commits that hopefully fix your issue(s).

SubDownloader cli can now be started in 3 modes:

• cli mode: subdownloader -c (the first subtitle will be selected)
• cli interactive mode: subdownloader -c -i (the user should select a subtitle)
• cli console mode: subdownloader -c -C (a console that allows more control)

The cli interactive mode is, I believe, equivalent to the old cli mode of subdownloader.

The current series of commands to download subtitles using the console is:

login
filescan
vidsearch
videos
vidselect 2
viddownload

The logic behind the renaming argument options is that it shows what will be the file name base.
Why do you feel this is unfortunate? Because your old scripts stop working or because someone might think that the script will rename the video?
Anyway, this should become clear when they read the output of subdownloader -h.

[mapreri]

Thanks for the detailed report!

That was from the reporter in the Debian bug, you should thank him!
So, I'm here forwarding also the reply now (after tweaking the pronouns and the formatting) :)

I agree it's underdocumented but its names are subject to change.
I just pushed some new commits that hopefully fix your issue(s).

SubDownloader cli can now be started in 3 modes:

• cli mode: subdownloader -c (the first subtitle will be selected)
• cli interactive mode: subdownloader -c -i (the user should select a subtitle)
• cli console mode: subdownloader -c -C (a console that allows more control)

The cli interactive mode is, I believe, equivalent to the old cli mode of subdownloader.

The -c / -c -i / -c -C flags described sound like they're providing all necessary options, and would fix my use-case.

But why don't -i and -C imply -c? I don't see how -g -i or -g -C would make sense.

The current series of commands to download subtitles using the console is:
[...]

The logic behind the renaming argument options is that it shows what will be the file name base.
Why do you feel this is unfortunate? Because your old scripts stop working or because someone might think that the script will rename the video?

I dislike the "--rename-video" flag because I'd expect it to rename a video.
I also think it's good form to keep a compatibility alias when renaming options, but that's a very minor issue in my use of subdownloader.

Quoting the -h output of subdownloader 2.1.0~rc4-1 for context:

|   --rename-online       Use the on-line subtitle filename as name for the
|                         downloaded subtitles. This is the default.
|   --rename-video        Use the local video filename as name for the
|                         downloaded subtitle.
|   --rename-lang         Use the local video filename + language as name for
|                         the downloaded subtitle.
|   --rename-uploader     Use the local video filename + uploader + language as
|                         name for the downloaded subtitle.

While I see what you want to express, I'd argue that from a user POV, there's no renaming going on here at all (especially for the --rename-online case!): files currently on the filesystem are untouched, and some new files are created.

How about calling them --name-online,--name-video,etc.? That avoids any implication that the user's (meta-)data might be at risk.

And while I'm here, can I suggest to change the default? I think writing to an effectively random name is the least useful option for an average user.

And since you wrote that the console command names are subject to change, I'd propose to rename

    vidsearch
    vidselect/viddeselect
    viddownload

They're not operating on videos. They're operating on subtitles, so s/vid/sub/g might make sense. But just going with

    search
    select/deselect
    download

should be just as clear, and easier to read and type.

[madebr]

But why don't -i and -C imply -c? I don't see how -g -i or -g -C would make sense.

Indeed, -i and -C only make sense in cli mode. They are thus documented in the cli section in the help. I've chosen to ignore these options when starting in gui mode.

How about calling them --name-online,--name-video,etc.? That avoids any implication that the user's (meta-)data might be at risk.

This naming convention makes sense. I guess I'll change it.

And while I'm here, can I suggest to change the default? I think writing to an effectively random name is the least useful option for an average user.

This seems reasonable. Using video+lang as default seems to be fine as a new default

And since you wrote that the console command names are subject to change, I'd propose to rename

    vidsearch
    vidselect/viddeselect
    viddownload

They're not operating on videos. They're operating on subtitles, so s/vid/sub/g might make sense. But just going with

    search
    select/deselect
    download

should be just as clear, and easier to read and type.

The current console also support searching for subtitles by text using the query* commands:

login
query the avengers
querymore
querymore
querymore
querysubsearch 0
querysubsearch 0
querysubsearch 0
querysubsearch 0
queryshow
language en
queryselect 0
querydownload

(there is some debug code in queryshow and a bug in querydownload)

I would also like to add upload support to the console.
It's not clear to me yet what would be a good design.
I'm thinking about splitting the console in the same 3 modes as the gui has: video/query/upload.

[mapreri]

But why don't -i and -C imply -c? I don't see how -g -i or -g -C would make sense.

Indeed, -i and -C only make sense in cli mode. They are thus documented in the cli section in the help. I've chosen to ignore these options when starting in gui mode.

I meant: I should not have to write subdownloader -c -i to get theinteractive CLI version, but just subdownloader -i. Ditto -C.

The current console also support searching for subtitles by text using the query* commands:

login
query the avengers
querymore
querymore
querymore
querysubsearch 0
querysubsearch 0
querysubsearch 0
querysubsearch 0
queryshow
language en
queryselect 0
querydownload

(there is some debug code in queryshow and a bug in querydownload)

I tried with 2883ff9:

| $ subdownloader -c -C
| SubDownloader 2.1.0rc4
| Type "help" for more information.
| >>> login
| Log in successful.
| >>> query the avengers
| >>> querymore
| >>> querymore
| >>> querysubsearch 0
| [2019-08-22 14:21:56,223] WARNING: subdownloader.languages.language # Unknown ISO639: ml
| [2019-08-22 14:21:56,225] WARNING: subdownloader.languages.language # Unknown ISO639: hi
| [2019-08-22 14:21:56,230] WARNING: subdownloader.languages.language # Unknown ISO639: hi
| [2019-08-22 14:21:56,232] WARNING: subdownloader.languages.language # Unknown ISO639: bn
| >>> querysubsearch 0
| [2019-08-22 14:22:05,141] WARNING: subdownloader.languages.language # Unknown ISO639: hi
| [2019-08-22 14:22:05,146] WARNING: subdownloader.languages.language # Unknown ISO639: ml
| [2019-08-22 14:22:05,152] WARNING: subdownloader.languages.language # Unknown ISO639: bn
| >>> queryshow
| [0] The Avengers (2012) [80/475]
|   [uk] 1 subtitle (Ukrainisch)
|     < > [0] Rating: 0.0, Uploader: igor911
|[...]
|   [zh] 1 subtitle (Chinese (traditional))
|     < > [78] Rating: 0.0, Uploader: unknown
|   [es] 1 subtitle (Spanisch (Spanien))
|     < > [79] Rating: 10.0, Uploader: cocospam
| [1] "The Avengers: Earth's Mightiest Heroes" The Ballad of Beta Ray Bill (2012) [0/28]
| [2] "The Avengers: Earth's Mightiest Heroes" Welcome to the Kree Empire (2012) [0/35]
|[...]
| [78] "The Avengers" The House That Jack Built (1966) [0/5]
| [79] "The Avengers" The Danger Makers (1966) [0/6]
| >>> language en
| *** Unknown syntax: language en
| >>> queryselect 0
| >>> querydownload
| Number of subs to download: 1
| - [UK] The_Avengers__2012__02h22m54s_23_976fps__Hurtom_.srt (148563 bytes) [opensubtitles]
| >>>
| $

querymore and querysubsearch don't seem to do anything for me, and language is not recognized at all. The latter is probably a good indication that you should allow it as an alias for languages. ;)
I can't suggest better names for the formers without knowing what they do; I'll just mention that having lots of commands prefixed the same is inconvenient for interactive use (while it preserves space for future extension, so this might be a valid tradeoff).

I would also like to add upload support to the console.
It's not clear to me yet what would be a good design.
I'm thinking about splitting the console in the same 3 modes as the gui has: video/query/upload.

I'm not sure about the console, but I think the cli interface should simply be subdownloader --upload en video.avi subtitle.sub.
I guess uploading multiple subs would be rather rare, so that builtin CLI support for that is not needed. (You can always write a for loop in shell to emulate it, or script the console interface.) And having it as an option invites accidents like uploading the 2nd video as the subtitle file of the first.

The --name-* options work and --name-lang is indeed the default, thanks!

[madebr]

Per your suggestion:

• the default action for -c is headless downloading of subtitles.
• --interactive will infer -c but is a bit interactive (you can select which subtitles to download)
• -C will start a fully interactive console (with commands)

The upload section is a WIP...

[mapreri]

Hi,

Do you think it would be possible for you to cut a rc5 release? There have been quite a few changes since rc4 :)

I also think this bug can be closed now, further improvements could be done through separate bugs (this is already messy enough as it is!)

[madebr]

I think I'll do a 2.1.0 release and start 2.2.0-dev because I have some significant backend changes queued.

[mapreri]

that works as well :) ("the version is just a string", etc)

…

On Thu, Oct 17, 2019 at 3:02 PM Anonymous Maarten ***@***.***> wrote: I think I'll do a 2.1.0 release and start 2.2.0-dev because I have some significant backend changes queued. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36?email_source=notifications&email_token=AAL7FE76EBFWM3GDOYWGOMLQPBO5XA5CNFSM4INV5OI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBQANKQ#issuecomment-543164074>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAL7FE33JJEWSCIO6HIXHELQPBO5XANCNFSM4INV5OIQ> .

-- regards, Mattia Rizzolo GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`. more about me: https://mapreri.org : :' : Launchpad user: https://launchpad.net/~mapreri `. `'` Debian QA page: https://qa.debian.org/developer.php?login=mattia `-

[madebr]

2.1.0 is available!

I'm closing this issue.
Please open a new issue when new problems arise!