-
-
Notifications
You must be signed in to change notification settings - Fork 517
Add search link type in the extractor #950
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
base: dev
Are you sure you want to change the base?
Conversation
Stypox
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks almost good to me. @FireMasterK could you take a look, too? Also, do we need tests for the link handler?
| // Utils | ||
| //////////////////////////////////////////////////////////////////////////*/ | ||
|
|
||
| public final String getIdByUrl(final String url) throws ParsingException{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of adding this function just to use it for search URLs in the client, I think a better option is what I suggested in the client review
| throw new ParsingException("the url given is not a YouTube-URL"); | ||
| } | ||
|
|
||
| final String listID = Utils.getQueryValue(urlObj, "search_query"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change listId to searchQuery and update the exception message accordingly
Honestly, I don't know how this part of the extractor works at all, so I would be the wrong candidate. We likely do need tests, since I did check the Channel link handler, which has tests. |
|
@Sola0404 do you plan to keep working on this? Also, you should also implement this for all other services, and not just for youtube, if possible (shouldn't be too difficult though). |
Uh oh!
There was an error while loading. Please reload this page.