Refactor lb-wrapper for command separation

[deldesir]

🚀 Pull Request Overview:

This pull request modified lb-wrapper to allow separate execution of 'tubeadd' and 'dl' commands. The script now takes a command type ('tubeadd' or 'dl') as the first argument, allowing users to choose which xklb command to run. Each command is executed independently with its own set of options.

This aims to facilitate the listing of requested files once lb tubeadd is done fetching metadata.

📋 Checklist:

• Tested the changes thoroughly.

🔗 Related Issue(s):
Issue #104

📌 Testing scenarios:
See Issue #97

cc @EMG70

PS This PR builds upon #109

[holta]

Personally I'd prefer these 2 right on the very top of the file, as the very first constants declared...

XKLB_CMD="$1"
URL="$2"

...to make the bash script more readable 🙏

[holta]

Or if necessary... ?

XKLB_INTERNAL_CMD="$1"
URL="$2"

And maybe preserve the original XKLB_FULL_CMD variable name alongside, to reduce confusion?

[deldesir]

XKLB_INTERNAL_CMD="$1"
URL="$2"

Using "$1" and appending the URL at the end makes the command ugly and difficult to grasp

And maybe preserve the original XKLB_FULL_CMD variable name alongside, to reduce confusion?

We can't per the design of separating them to decouple metadata fetching and video downloading

[holta]

It sounds like you're not understanding my suggestions.

I'll try to reach you privately.

[deldesir]

Thanks for explaining. Done.

[holta]

Special Request: Please keep all "CONSTANTS" near the top of the bash script (this one especially should stay where it is around Line 13, with the English explanation about alternatives for OOM debugging etc!)

FORMAT_OPTIONS="--format best --format-sort 'tbr~1000'"

[deldesir]

Done

[holta]

Please (re)delete lines 55-58 per JC suggestion (:

As was merged with PR #109 here:

• f9a9084

[deldesir]

Done

[holta]

Validation on Lines 43-44 should definitely be restored in my opinion:

if ! command -v "${XKLB_EXECUTABLE}"; then
    log "Error" "xklb could not be found. Please install xklb and try again."

(...followed by a blank line if possible!)

[deldesir]

Done

[holta]

@deldesir this is getting closer.

1. Please remove Lines 16-18:

   • 322c440#r137829261

   (As XKLB_FULL_CMD is definitely no longer a "constant" — xklb_full_cmd is now a variable that should clearly be set [within if / else] to one-value-or-another.)

2. Please clarify where lb-wrapper is invoked and how that will need to be revised.

[holta]

1. @EMG70 please install all 4 PR's together:

   • PR Refactor lb-wrapper for command separation #110
   • PR Decouple the metadata fetching with TaskMetadataExtract #111
   • PR Create cps/tasks/metadata_extract.py #112
   • PR Call download task from TaskMetadataExtract #113

   ...onto a fresh Multipass VM running Ubuntu 24.04 (latest pre-release).

2. Then please Test Scenarios 1., 2. and 3. especially (from Testing scenarios for multimedia downloads using iiab/calibre-web [TDD, Gherkin Stories] #97, screenshot below!) Do not expect 4. and 5. to work well just yet!

[holta]

Thank you @EMG70 and @deldesir for careful TDD (focused testing) over the past week.

Looking to the future now with these PRs are others:

• PR Make URLs clickable [in "Tasks" view] #115
• PR Retrieve relevant description #116