Skip to content

Conversation

@imatikshaikh
Copy link

This PR makes the following changes:

  • Shows interactive Program UI after downloads complete for all runs, not just when started with --download
  • Updates director.py to present UI options after any download run completes
  • User can perform additional actions without restarting the program
  • Documentation and CHANGELOG updated to reflect changes

@jbsparrow
Copy link
Owner

It does not show the UI post-run when using --download.

This is a nice idea but UI should never be shown when using --download. I'm not sure why yours does that but an issue should be opened regarding it.

I also think there should be different UI post-download with options such as hashing the current download run, deduplicating, retrying failed downloads/scrapes from the current run, etc.

Copy link
Collaborator

@NTFSvolume NTFSvolume left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this behavior. This means CDL will now always need human interaction to exit. You will not be able to use it in a script or any kind of automation.

It would be trivial for the user to just run CDL in an infinite loop if the want to see the UI again after a run.

At minimum it should be ignored with --download or any of the other options that disables the UI (all the retry options, the no ui options and simplified ui options)

Comment on lines +115 to +134
# Always show the interactive Program UI after downloads finish so the user
# can continue (for example re-run downloads or edit configs). If the user
# selects the download action in the UI we re-insert the current config
# at the front of `configs_to_run` to run it again.
if not manager.states.SHUTTING_DOWN.is_set():
try:
ui = ProgramUI(manager, run=False)
# Run the blocking UI code in a thread so it doesn't interfere with the
# manager's asyncio event loop. This prevents prompt_toolkit/Application
# coroutine warnings when called from an active loop.
ui_result = await asyncio.to_thread(ui.run)
except SystemExit:
# user chose to exit from the UI
raise
except Exception:
# if UI fails for any reason, do not crash the director — continue shutdown
ui_result = False
else:
if ui_result:
configs_to_run.insert(0, current_config)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break if the user switched to a new config on the new UI or created a new one.

I would rather leave it as is, completely destroy the current manager and re-run the director from scratch

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There also needs to be an "enter to continue" just before that to make sure the stats show up on the terminal

Comment on lines +38 to +49
- `ProgramUI.run()` is now invoked from the director using a thread (`asyncio.to_thread`)
to avoid prompt_toolkit coroutine warnings when running inside the program's async
runtime.

- `cyberdrop_dl.__init__` now falls back to a local version string when distribution
metadata is not available. This makes running from a development checkout more
convenient (no editable install required to start the program).

### Fixed

- Prevent a "coroutine was never awaited" RuntimeWarning when invoking the TUI from
the running event loop.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are too specific

Suggested change
- `ProgramUI.run()` is now invoked from the director using a thread (`asyncio.to_thread`)
to avoid prompt_toolkit coroutine warnings when running inside the program's async
runtime.
- `cyberdrop_dl.__init__` now falls back to a local version string when distribution
metadata is not available. This makes running from a development checkout more
convenient (no editable install required to start the program).
### Fixed
- Prevent a "coroutine was never awaited" RuntimeWarning when invoking the TUI from
the running event loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants