Skip to content

Add missing case when trying to parse video ID - #876

Merged
Tyrrrz merged 3 commits into
Tyrrrz:masterfrom
ngdph-forks:fixes
Mar 12, 2025
Merged

Tyrrrz merged 3 commits into
Tyrrrz:masterfrom
ngdph-forks:fixes

Conversation

@ngdph

@ngdph ngdph commented Mar 11, 2025

Copy link
Copy Markdown
Contributor

This PR only adds a missing case when trying to parse the video ID. I ran into it when using this video link:
https://youtu.be/watch?v=Fcds0_MrgNU

@Tyrrrz Tyrrrz added the bug label Mar 11, 2025
@Tyrrrz
Tyrrrz requested a review from Copilot March 11, 2025 19:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a missing case for parsing YouTube video IDs from shortened URL formats.

Comments suppressed due to low confidence (2)

YoutubeExplode/Videos/VideoId.cs:66

  • The regular expression used to extract the video ID is quite permissive and may not reliably capture the correct ID in all cases. Consider tightening the pattern by explicitly matching the expected 11-character ID pattern and applying RegexOptions.IgnoreCase to handle potential case variations.
.Match(videoIdOrUrl, @"youtu\.be/watch.*?v=(.*?)(?:\?|&|/|$)")

YoutubeExplode/Videos/VideoId.cs:70

  • Since this new extraction branch introduces additional URL parsing, please ensure that there are corresponding tests covering various shortened URL formats (including edge cases with extra query parameters) to verify its robustness.
if (!string.IsNullOrWhiteSpace(id) && IsValid(id))

Comment thread YoutubeExplode/Videos/VideoId.cs Resolved
@Tyrrrz Tyrrrz changed the title Add missing case when trying to parse video id Add missing case when trying to parse video ID Mar 11, 2025
@Tyrrrz

Tyrrrz commented Mar 11, 2025

Copy link
Copy Markdown
Owner

Hi, thanks for the PR. Can you please:

  1. Move the newly added code block above the (shortened) block.
  2. Add test data for this in
    [Theory]
    [InlineData("youtube.com/watch?v=yIVRs6YSbOM", "yIVRs6YSbOM")]
    [InlineData("youtu.be/yIVRs6YSbOM", "yIVRs6YSbOM")]
    [InlineData("youtube.com/embed/yIVRs6YSbOM", "yIVRs6YSbOM")]
    [InlineData("youtube.com/shorts/sKL1vjP0tIo", "sKL1vjP0tIo")]
    [InlineData("youtube.com/live/jfKfPfyJRdk", "jfKfPfyJRdk")]

@ngdph

ngdph commented Mar 12, 2025

Copy link
Copy Markdown
Contributor Author

Yes sir, I've done it.

Hi, thanks for the PR. Can you please:

  1. Move the newly added code block above the (shortened) block.
  2. Add test data for this in
    [Theory]
    [InlineData("youtube.com/watch?v=yIVRs6YSbOM", "yIVRs6YSbOM")]
    [InlineData("youtu.be/yIVRs6YSbOM", "yIVRs6YSbOM")]
    [InlineData("youtube.com/embed/yIVRs6YSbOM", "yIVRs6YSbOM")]
    [InlineData("youtube.com/shorts/sKL1vjP0tIo", "sKL1vjP0tIo")]
    [InlineData("youtube.com/live/jfKfPfyJRdk", "jfKfPfyJRdk")]

@Tyrrrz

Tyrrrz commented Mar 12, 2025

Copy link
Copy Markdown
Owner

Cool, thanks!

@Tyrrrz
Tyrrrz requested a review from Copilot March 12, 2025 19:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses an issue by adding logic to extract a YouTube video ID from a partially shortened URL format.

  • Added extraction logic for URLs matching the "youtu.be/watch?v=" pattern.
  • Updated tests to cover the new URL pattern.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
YoutubeExplode/Videos/VideoId.cs Added new URL extraction branch to parse video IDs from URLs containing "watch?v="
YoutubeExplode.Tests/VideoIdSpecs.cs Introduced a new InlineData test case for the added URL pattern

Comment thread YoutubeExplode/Videos/VideoId.cs Resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants