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

feat(youtube): Refactor YouTube matching logic #1466

Merged
merged 4 commits into from
Mar 15, 2023

Conversation

Wolveric
Copy link
Contributor

This PR is aimed at updating elements of the URL parsing logic for the YouTube feed, by adding checks of the URL path in order to streamline parsing logic, as well as raise more relevant, context-sensitive errors.

Additionally, these changes come alongside using these changes to add support for URLs using the shorts path in the process.

This commit redesigns `Plugin.parseYtUrl`, making use of the
`net/url.Url` struct to extract out key elements of YouTube URLs,
minimising the utilisation of RegEx in the method, alongside simplifying
remaining expressions.

Additionally, this adds the `Plugin.parseYtVideoID`
method, to extract some of the parsing logic out of `Plugin.parseYtUrl`,
and introduces support for URLs to YouTube shorts.
Updates the signature of `Plugin.getYtChannel`, to work the the
redesigned `Plugin.parseYtUrl` method.
Updates the `Plugin.HandleNew` to use `net/url.Url`, inline with the new
signature of `Plugin.getYtChannel`. This includes updating some variable
names, to avoid blocking the use of the `net/url` package.
@ashishjh-bst ashishjh-bst changed the title feat(youtube): Redesign YouTube matching logic feat(youtube): Refactor YouTube matching logic Mar 14, 2023
@Wolveric
Copy link
Contributor Author

I should note: I think the domain checking logic being handled by Plugin.HandleNew is a bit funky with these changes, as it effectively results in the check for the youtu.be domain being duplicated, due to being a parsing check in Plugin.parseYtUrl, but I left it as it stands, to satisfy the existing error reporting logic...

If anyone has a suggestion in how to rewrite the error reporting, I'd like to add that as another change...

Updates `Plugin.parseYtUrl`, using `fmt.Errorf` to include the URL
element, which resulted in it failing to be parsed.
@ashishjh-bst ashishjh-bst merged commit 8108f5b into botlabs-gg:dev Mar 15, 2023
ashishjh-bst added a commit to ashishjh-bst/yagpdb that referenced this pull request Mar 15, 2023
ashishjh-bst added a commit that referenced this pull request Mar 15, 2023
Co-authored-by: Ashish Jhanwar <[email protected]>
@Wolveric Wolveric deleted the rework-yt-matching-logic branch March 22, 2023 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants