update parse_time in misc.py by igel-kun · Pull Request #2563 · pyload/pyload · GitHub
Skip to content

update parse_time in misc.py#2563

Open
igel-kun wants to merge 2 commits into
pyload:stablefrom
igel-kun:misc
Open

update parse_time in misc.py#2563
igel-kun wants to merge 2 commits into
pyload:stablefrom
igel-kun:misc

Conversation

@igel-kun

@igel-kun igel-kun commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

added functionality to parse times in format HH:MM:SS and MM:SS

@igel-kun

igel-kun commented Aug 1, 2016

Copy link
Copy Markdown
Contributor Author

seconds = seconds_to_midnight()

elif re.search("\d:\d\d", value):
# use the HH:MM:SS format, NOTE: when only one ':' is found, it assumes MM:SS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why assume MM:SS and not HH:MM?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, there is no way around assuming one of the two without context, and seeing that I stumbled upon that while handling wait times (they were always below 1h), this seemed the obvious choice. However, to be more general, maybe an argument should be added to the function telling it how to interprete a single colon.

@vuolter vuolter added enhancement New feature or bugfix help wanted Extra attention is needed labels Oct 23, 2016
@vuolter vuolter added this to the 0.5.x milestone Oct 23, 2016
@vuolter vuolter added invalid This doesn't seem right feedback wanted User attention is needed and removed help wanted Extra attention is needed labels Dec 27, 2018
@CLAassistant

CLAassistant commented Aug 27, 2019

Copy link
Copy Markdown

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

Labels

enhancement New feature or bugfix feedback wanted User attention is needed invalid This doesn't seem right

Development

Successfully merging this pull request may close these issues.

4 participants