Skip to content

Conversation

@Sola0404
Copy link

@Sola0404 Sola0404 commented Oct 23, 2022

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

Copy link
Member

@Stypox Stypox left a 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{
Copy link
Member

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");
Copy link
Member

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

@FireMasterK
Copy link
Member

Looks almost good to me. @FireMasterK could you take a look, too? Also, do we need tests for the link handler?

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.

@Stypox
Copy link
Member

Stypox commented Mar 28, 2024

@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).

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.

3 participants