search

lay295 / TwitchDownloader

Twitch VOD/Clip Downloader - Chat Download/Render/Replay
MIT License
2.52k stars 259 forks source link

Add warning before overwrite file #1034

Closed atomsbaza closed 3 weeks ago

atomsbaza commented 2 months ago

TODO:

Feedback is welcomed.

ScrubN commented 2 months ago

Thank you for the PR, however there are several issues:

  1. This will only work for the CLI. As it stands, this would completely break the Windows GUI
  2. Continuing on y, else failing is poor design in my opinion. The user should be explicitly required to confirm or reject, otherwise the program should ask again.
  3. How will this work for 3rd party libraries that wrap TDCLI? They cannot reasonably work around this.
  4. How will this work with status disabled? (--log-level None)
  5. What about chat updater and chat renderer?
atomsbaza commented 2 months ago

Thank you for the PR, however there are several issues:

  1. This will only work for the CLI. As it stands, this would completely break the Windows GUI
  2. Continuing on y, else failing is poor design in my opinion. The user should be explicitly required to confirm or reject, otherwise the program should ask again.
  3. How will this work for 3rd party libraries that wrap TDCLI? They cannot reasonably work around this.
  4. How will this work with status disabled? (--log-level None)
  5. What about chat updater and chat renderer?

Thank you for your feedback. To answer question

  1. For Windows GUI I think it isn't needed because Windows already asks to confirm before saving the file but I'm not sure that will cover all necessary because I'm using macOS so I will look for a solution again.
  2. I'm gonna improve it as you ask.
  3. I'm gonna looking ways to make it work.
  4. I will add more logs that can be used from ITaskLogger.
  5. I will add a warning for chat updater and chat rederer
ScrubN commented 2 months ago
  1. For Windows GUI I think it isn't needed because Windows already asks to confirm before saving the file but I'm not sure that will cover all necessary because I'm using macOS so I will look for a solution again.

The Windows save file dialog does not delete files when a user clicks "yes". Because of that, your changes break the Windows GUI completely due to scanning Console.In. rider64_wmqsy6BIuZ (1)

atomsbaza commented 2 months ago
  1. For Windows GUI I think it isn't needed because Windows already asks to confirm before saving the file but I'm not sure that will cover all necessary because I'm using macOS so I will look for a solution again.

The Windows save file dialog does not delete files when a user clicks "yes". Because of that, your changes break the Windows GUI completely due to scanning Console.In. rider64_wmqsy6BIuZ (1) rider64_wmqsy6BIuZ (1)

I see what happen now. Thank you for your feedback will work in-progress

atomsbaza commented 1 month ago

@ScrubN could you please explain more about the status disabled (--log-level None)

ScrubN commented 1 month ago

When --log-level None is passed, nothing is written to stdout except for uncaught exceptions.

atomsbaza commented 1 month ago

Thanks, @ScrubN Could you please review this PR after I complete my changes?