Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Heise] Fixes #14961 #15026

Closed
wants to merge 10 commits into from
Closed

[Heise] Fixes #14961 #15026

wants to merge 10 commits into from

Conversation

Hier631
Copy link

@Hier631 Hier631 commented Dec 18, 2017

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

The extractor for heise.de is not longer working so I thought that I could use the mobile version of the site to download the videos.

@@ -54,6 +55,9 @@ class HeiseIE(InfoExtractor):
}]

def _real_extract(self, url):
url = re.sub(r'https?://(?:www\.|m\.)?heise\.de/',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should match from the beginning. It's senseless to replace mobile URL with itself.

@Hier631
Copy link
Author

Hier631 commented Dec 19, 2017

It's OK now?

@dstftw
Copy link
Collaborator

dstftw commented Dec 19, 2017

Should match from the beginning.

@Hier631
Copy link
Author

Hier631 commented Dec 19, 2017

What do you mean?

@Hier631
Copy link
Author

Hier631 commented Dec 19, 2017

Ok, I understand it

@Hier631
Copy link
Author

Hier631 commented Dec 19, 2017

It's OK now?

@@ -54,6 +55,9 @@ class HeiseIE(InfoExtractor):
}]

def _real_extract(self, url):
url = re.sub(r'^https?://(?:www\.)?heise\.de/',
'https://m.heise.de/',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep the original scheme.

@@ -54,6 +55,9 @@ class HeiseIE(InfoExtractor):
}]

def _real_extract(self, url):
url = re.sub(r'^https?://(?:www\.)?heise\.de/',
'https://m.heise.de/',
url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove unnecessary verbosity.

@@ -8,11 +8,12 @@
int_or_none,
parse_iso8601,
xpath_text,
re,
Copy link
Collaborator

Choose a reason for hiding this comment

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

re is not from utils.

@@ -54,6 +55,9 @@ class HeiseIE(InfoExtractor):
}]

def _real_extract(self, url):
url = re.sub(r'^https?://(?:www\.)?heise\.de/',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't shadow existing name.

@Hier631
Copy link
Author

Hier631 commented Dec 19, 2017

It's OK now?

@dstftw
Copy link
Collaborator

dstftw commented Dec 20, 2017

Tests broken.

@Hier631
Copy link
Author

Hier631 commented Dec 20, 2017

Tests fixed.

@dstftw
Copy link
Collaborator

dstftw commented Dec 21, 2017

Does not work for new videos: https://www.heise.de/video/artikel/odcast-c-t-uplink-20-1-
Apple-CarPlay-vs-Android-Auto-Galileo-3D-Sound-erklaert-3919694.html

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

Successfully merging this pull request may close these issues.

2 participants