Added support for specifying minimum skip length #169
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
src/iSponsorBlockTV/config_setup.py
Outdated
| # Ask for minimum skip length. Confirm input is an integer | ||
| while True: | ||
| try: | ||
| minimum_skip_length = int( | ||
| input("Enter minimum length of segment to skip in seconds: ") | ||
| ) | ||
| break | ||
| except ValueError: | ||
| print("You entered a non integer value, try again.") | ||
| continue | ||
| config.minimum_skip_length = minimum_skip_length | ||
|
|
There was a problem hiding this comment.
PR #150 was merged probably between when you raised this one and now which refactored this script a bit. It should be pretty simple to adapt this to use the get_yn_input function - though truthfully this is likely fine as is. But you will need to pull the changes for the rest of the script to merge without conflict.
It also might be helpful for the user to include something like "set this to 0 to not skip any segments" or something to that effect.
There was a problem hiding this comment.
Yep, #150 was merged after raising this PR, but refactoring this section of the cli configurator shouldn't be too much work, and ideally a new "panel" would be added to the tui config editor to allow changing this value from there too. I'll be able to do it at some point but if someone wants to go ahead and help you're free to do it. PRs are always appreciated. Thanks for your comment @ryankupk
src/iSponsorBlockTV/helpers.py
Outdated
| @@ -41,6 +41,7 @@ def __init__(self, data_dir): | |||
| self.skip_count_tracking = True | |||
| self.mute_ads = False | |||
| self.skip_ads = False | |||
| self.minimum_skip_length = 1 | |||
There was a problem hiding this comment.
I believe that the minimum ought to be 0 rather than 1, since sponsorblock segments are defined by frames so there could be a segment that's less than 1 second that would be skipped when the user might think that no skipping is happening.
Although one of the configurators will likely always be ran, so the user will define a minimum segment length, I still think that ideally the default would represent no skipping.
There was a problem hiding this comment.
I find 1 to be an appropriate minimum time length, considering that YouTube only allows setting the play head to integer values (so no 10.8 seconds allowed, it'll just round it down to 10). If a user were to want to disable skipping, they should be able to just set the list of skippable categories to an empty one (that should work but it isn't something I've tested)
…o 0 so user has to explicitly enable functionality
for more information, see https://pre-commit.ci
|
I have merged all changes, updated the setup to match the new format, and set default value of skip length to 0 so a user has to explicitly enable the functoinality |
Fixes #145
This provides support to specify minimum segment length to skip
I have no chance of updating the graphical configurator and having it work correctly, I hope the utility provided in the rest of this pull request makes up for it