This repository has been archived by the owner on Dec 3, 2023. It is now read-only.
fix for failed download with no subtitles when requested #37
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
At the moment if video doesn't have subtitles `youtube-dl` fails with ...
```js
events.js:72
throw er; // Unhandled 'error' event
^
Error: : video doesn't have subtitles
at /test/youtube-dl/lib/youtube-dl.js:122:23
at ChildProcess.exithandler (child_process.js:645:7)
at ChildProcess.emit (events.js:98:17)
at maybeClose (child_process.js:755:16)
at Process.ChildProcess._handle.onexit (child_process.js:822:5)
```
... this fix catches and removes possible options passed by the user and tries once to download video only without the subtitles.
| @@ -71,6 +71,7 @@ var ytdl = module.exports = function(url, args, options) { | |||
| * @param {Function(!Error, String)} callback | |||
| */ | |||
| function call(video, args1, args2, options, callback) { | |||
|
|
|||
There was a problem hiding this comment.
not sure why these new lines are being placed at the start of blocks?
Cleanup, traverse the list of args backwards, match all possible languages, regex in global var
| */ | ||
| function reIndexOf(arr, rx) { | ||
| var i; | ||
| for (i = 0; i < arr.length; i++) { |
There was a problem hiding this comment.
Iterate backwards, should look like
for (i = arr.length - 1; i >= 0; i--) {
There was a problem hiding this comment.
Already reversed L124
There was a problem hiding this comment.
Reversing the array wouldn't fix the issue. The issue with iterating it in incremental order is that once you process an item and remove it from the current index, the length of the array decreases. The item in the next index will be moved to the current index, but i will still increase, skipping that item.
| function reIndexOf(arr, rx) { | ||
| var i; | ||
| for (i = arr.length - 1; i >= 0; i--) { | ||
| if (arr[i].toString().match(rx)) { return i; } |
There was a problem hiding this comment.
why not use RegExp#test() instead, and create the regexp objects beforehand?
this should do it ...
fent
added a commit
that referenced
this pull request
Aug 2, 2014
fix for failed download with no subtitles when requested
|
Thank you again! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At the moment if video doesn't have subtitles but they were requested by the user
youtube-dlfails with ...events.js:72 throw er; // Unhandled 'error' event ^ Error: : video doesn't have subtitles at /test/youtube-dl/lib/youtube-dl.js:122:23 at ChildProcess.exithandler (child_process.js:645:7) at ChildProcess.emit (events.js:98:17) at maybeClose (child_process.js:755:16) at Process.ChildProcess._handle.onexit (child_process.js:822:5)... this fix catches and removes possible options passed by the user and tries once to download video only without the subtitles.