Add subscriber_count to the author object. #606
Conversation
Codecov Report
@@ Coverage Diff @@
## master #606 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 7 7
Lines 552 561 +9
=========================================
+ Hits 552 561 +9
Continue to review full report at Codecov.
|
lib/util.js
Outdated
| @@ -348,9 +348,30 @@ exports.addFormatMeta = format => { | |||
| * @param {string} html | |||
| * @returns {string} | |||
| */ | |||
| exports.stripHTML = html => html | |||
| exports.stripHTML = html => decodeURIComponent(html) | |||
There was a problem hiding this comment.
I'm getting a URIError: URI malformed error from this
try calling it after this function is called
lib/util.js
Outdated
| if (string.match(/[0-9,.]*[M]/)) { | ||
| return parseFloat(string.replace('M', '')) * 1000000; | ||
| } else if (string.match(/[0-9,.]*[K]/)) { | ||
| return parseFloat(string.replace('K', '')) * 1000; | ||
| } else { | ||
| return parseFloat(string); | ||
| } |
There was a problem hiding this comment.
would look simpler if the number + the multiplier were extracted together. something like
let match = string.match(/([\d,.]+)([MK]?)/);
if (match) {
let [num, multi] = match;
}
lib/info-extras.js
Outdated
| @@ -100,6 +104,7 @@ exports.getAuthor = body => { | |||
| user: username, | |||
| channel_url: `https://www.youtube.com/channel/${channelID}`, | |||
| user_url: `https://www.youtube.com/user/${username}`, | |||
| subscriber_count: isNaN(subscriberCount) ? null : subscriberCount, | |||
There was a problem hiding this comment.
move this check to inside the parseAbbreviatedNumber() function
There was a problem hiding this comment.
Thank you for your feedback.
The abbreviated number now gets matched once, and number and multiplier are extracted from the second and third match group.
Also, if it doesn't match the regex, the function returns null. The isNaN check is therefore not necessary anymore.
I also moved the decodeURIComponent outside of the stripHTML function.
lib/util.js
Outdated
| .match(/([\d,.]+)([MK]?)/); | ||
| if (match) { | ||
| const [, num, multi] = match; | ||
| return multi === 'M' ? +num * 1000000 : | ||
| multi === 'K' ? +num * 1000 : +num; | ||
| } | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
The abbreviated number now gets matched once, and number and multiplier are extracted from the second and third match group.
Also, if it doesn't match the regex, the function returns null. The isNaN check is therefore not necessary anymore.
lib/util.js
Outdated
| return multi === 'M' ? +num * 1000000 : | ||
| multi === 'K' ? +num * 1000 : +num; |
lib/info-extras.js
Outdated
| @@ -18,7 +18,10 @@ const getMetaItem = (body, name) => util.between(body, `<meta itemprop="${name}" | |||
| exports.getVideoDescription = html => { | |||
| const regex = /<p.*?id="eow-description".*?>(.+?)<\/p>[\n\r\s]*?<\/div>/im; | |||
| const description = html.match(regex); | |||
| return description ? Entities.decode(util.stripHTML(description[1])) : ''; | |||
| if (description) { | |||
| return Entities.decode(decodeURIComponent(util.stripHTML(description[1]))); | |||
There was a problem hiding this comment.
hmm, still getting the decode error with this video https://www.youtube.com/watch?v=PGqU_stcXDw
are you trying to fix URLs in descriptions? if so, then you can give the .replace calls inside stripHTML a function https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#Specifying_a_function_as_a_parameter
also, it would be out of this scope of this PR
lib/info-extras.js
Outdated
| if (description) { | ||
| return Entities.decode(decodeURIComponent(util.stripHTML(description[1]))); | ||
| } | ||
| return ''; | ||
| return description ? Entities.decode(util.stripHTML(description[1])) : ''; |
There was a problem hiding this comment.
I reverted the changes on description, and will open a separate PR with a working version.
There was a problem hiding this comment.
cool. you can create another branch on your fork. typically, people create branches for each of their features/fixes, that way you can work on many at the same time without having to wait for one of them to merge
| let [, num, multi] = match; | ||
| num = parseFloat(num); | ||
| return multi === 'M' ? num * 1000000 : | ||
| multi === 'K' ? num * 1000 : num; |
There was a problem hiding this comment.
Replaced unary + with parseFloat.
Add the subscriber count to the author object.
It gets converted into a number from an abbreviated form.
Support article on the abbreviation: https://support.google.com/youtube/thread/6543166?hl=en&msgid=13119244