Add PowerKit and JsonExtensions; remove duplicate local utilities/extensions#948
Add PowerKit and JsonExtensions; remove duplicate local utilities/extensions#948
Conversation
Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/fc4c8d27-c827-4ffe-adfa-3b45e035cd79 Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
|
@copilot did you also check the other projects? |
Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/0bf38e9d-95a8-4565-8833-820c6f9aa2d2 Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Good catch — missed the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot what about the test projects? |
Checked both
The two |
|
@copilot TempDir/TempFile definitely exist in POwerKit |
…erKit's TempFile/TempDirectory) Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/2ac202dd-44e8-4aec-905c-ed2d224c3cc3 Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in 1d0a035. Added |
|
@copilot is SwapChars called more than in one place? If it's only used in one place, then inline it. |
Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/54ddafe9-8258-4a16-a108-a04d7965ba61 Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
|
If we added the JsonExtensions NuGet package, would we be able to remove most of this class? |
…eep only EnumerateDescendantProperties Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/926716d3-a2c0-4531-9e8d-72cf7a7f037e Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Yes — the Done in 4fe7e12: added |
|
@copilot any obvious refactorings that can be done with the newly added |
Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/d7ecb3f3-0823-4482-aa9d-4c8dffc2989c Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in 71a7b9f. The most obvious cleanup was in The rest of the codebase was already making good use of |
|
@copilot any other PowerKit-provided extensions/utilities that might be useful? |
|
@copilot update PowerKit, it adds some new APIs that may be useful to YoutubeExplode |
Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/f3f87da8-cc96-49a0-ac22-283a67f732dd Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in 01a09c9. Two more improvements applied:
Everything else already uses the relevant PowerKit APIs ( |
…it.ProgressMuxer Agent-Logs-Url: https://github.com/Tyrrrz/YoutubeExplode/sessions/fb1c514f-6839-4a0e-a532-db9a310b522b Co-authored-by: Tyrrrz <1935960+Tyrrrz@users.noreply.github.com>
Done in ad9bdd5. Updated to The most significant new addition is The other new files ( Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR consolidates duplicated internal utility/extension code across the solution by switching call sites to the PowerKit and JsonExtensions NuGet packages, and removes the now-redundant local implementations.
Changes:
- Replaced many local
Utils/Extensionshelpers withPowerKit.Extensions/JsonExtensions.Readingand deleted the corresponding local source files. - Updated parsing/call-site code to use PowerKit helpers (e.g.,
ParseOrNull,Clamp, whitespace filters). - Updated central package versions (incl. new
PowerKit/JsonExtensionsreferences and prerelease bumps).
Reviewed changes
Copilot reviewed 71 out of 71 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| YoutubeExplode/YoutubeHttpHandler.cs | Switches hashing + handler base to PowerKit-based helpers. |
| YoutubeExplode/YoutubeExplode.csproj | Adds JsonExtensions and PowerKit package refs for the main library. |
| YoutubeExplode/Videos/VideoId.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Videos/VideoController.cs | Moves JSON reading helpers to JsonExtensions.Reading. |
| YoutubeExplode/Videos/Streams/VideoQuality.cs | Uses PowerKit parsing helpers. |
| YoutubeExplode/Videos/Streams/StreamClient.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Videos/ClosedCaptions/ClosedCaptionController.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Videos/ClosedCaptions/ClosedCaptionClient.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Utils/Xml.cs | Uses PowerKit XElement extensions. |
| YoutubeExplode/Utils/UrlEx.cs | Uses PowerKit extensions. |
| YoutubeExplode/Utils/Hash.cs | Removes local hash helper. |
| YoutubeExplode/Utils/Extensions/XElementExtensions.cs | Removes local XML namespace stripping extension. |
| YoutubeExplode/Utils/Extensions/UriExtensions.cs | Removes local URI domain extension. |
| YoutubeExplode/Utils/Extensions/StringExtensions.cs | Removes duplicated string helpers, keeps only StripNonDigit. |
| YoutubeExplode/Utils/Extensions/StreamExtensions.cs | Removes local stream progress copy helper. |
| YoutubeExplode/Utils/Extensions/JsonExtensions.cs | Keeps only YouTube-specific recursive JSON search; relies on JsonExtensions for the rest. |
| YoutubeExplode/Utils/Extensions/HttpExtensions.cs | Removes local HTTP request cloning / HEAD helper. |
| YoutubeExplode/Utils/Extensions/GenericExtensions.cs | Removes local Pipe helper (covered by PowerKit). |
| YoutubeExplode/Utils/Extensions/CollectionExtensions.cs | Removes local collection helpers (covered by PowerKit). |
| YoutubeExplode/Utils/Extensions/AsyncCollectionExtensions.cs | Removes local async enumerable helpers (covered by PowerKit). |
| YoutubeExplode/Utils/ClientDelegatingHandler.cs | Removes local delegating handler (replaced by PowerKit). |
| YoutubeExplode/Search/SearchClient.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Playlists/PlaylistId.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Common/IBatchItem.cs | Moves async enumerable helpers to PowerKit. |
| YoutubeExplode/Common/Batch.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Channels/UserName.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Channels/ChannelSlug.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Channels/ChannelId.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Channels/ChannelHandle.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Channels/ChannelClient.cs | Uses PowerKit parsing helpers. |
| YoutubeExplode/Bridge/VideoWatchPage.cs | Uses JsonExtensions + PowerKit parsing/whitespace helpers. |
| YoutubeExplode/Bridge/ThumbnailData.cs | Moves JSON reading helpers to JsonExtensions. |
| YoutubeExplode/Bridge/SearchResponse.cs | Uses JsonExtensions + PowerKit helpers in search response model parsing. |
| YoutubeExplode/Bridge/PlaylistVideoData.cs | Uses JsonExtensions + PowerKit parsing helpers. |
| YoutubeExplode/Bridge/PlaylistNextResponse.cs | Uses JsonExtensions + PowerKit parsing helpers. |
| YoutubeExplode/Bridge/PlaylistBrowseResponse.cs | Uses JsonExtensions + PowerKit parsing helpers. |
| YoutubeExplode/Bridge/PlayerSource.cs | Moves extension usage to PowerKit. |
| YoutubeExplode/Bridge/PlayerResponse.cs | Uses JsonExtensions + PowerKit parsing helpers. |
| YoutubeExplode/Bridge/DashManifest.cs | Uses PowerKit parsing helpers. |
| YoutubeExplode/Bridge/ClosedCaptionTrackResponse.cs | Uses PowerKit XElement/string helpers. |
| YoutubeExplode/Bridge/Cipher/SwapCipherOperation.cs | Inlines swap logic instead of using removed string helper. |
| YoutubeExplode/Bridge/Cipher/ReverseCipherOperation.cs | Uses PowerKit string reverse extension. |
| YoutubeExplode/Bridge/ChannelPage.cs | Moves extension usage to PowerKit. |
| YoutubeExplode.Tests/YoutubeExplode.Tests.csproj | Adds PowerKit for temp file/dir utilities used by tests. |
| YoutubeExplode.Tests/Utils/TempFile.cs | Removes local temp file helper (replaced by PowerKit). |
| YoutubeExplode.Tests/StreamSpecs.cs | Switches temp file usage to PowerKit. |
| YoutubeExplode.Tests/ClosedCaptionSpecs.cs | Switches temp file usage to PowerKit. |
| YoutubeExplode.Demo.Gui/YoutubeExplode.Demo.Gui.csproj | Adds PowerKit dependency for GUI demo utilities. |
| YoutubeExplode.Demo.Gui/ViewModels/MainViewModel.cs | Switches filename sanitization and progress helper to PowerKit. |
| YoutubeExplode.Demo.Gui/Utils/Extensions/PathExtensions.cs | Removes local path sanitization helper. |
| YoutubeExplode.Demo.Gui/Utils/DelegateProgress.cs | Removes local progress implementation (replaced by PowerKit). |
| YoutubeExplode.Converter/YoutubeExplode.Converter.csproj | Adds PowerKit for converter utilities. |
| YoutubeExplode.Converter/Utils/ProgressMuxer.cs | Removes local progress muxer (replaced by PowerKit). |
| YoutubeExplode.Converter/Utils/Extensions/StringExtensions.cs | Removes local string extensions (covered by PowerKit). |
| YoutubeExplode.Converter/Utils/Extensions/LanguageExtensions.cs | Moves extension usage to PowerKit. |
| YoutubeExplode.Converter/Utils/Extensions/GenericExtensions.cs | Removes local Pipe helper (covered by PowerKit). |
| YoutubeExplode.Converter/Utils/Extensions/AsyncCollectionExtensions.cs | Removes local async enumerable helpers (covered by PowerKit). |
| YoutubeExplode.Converter/FFmpeg.cs | Uses PowerKit Clamp extension. |
| YoutubeExplode.Converter/Converter.cs | Uses PowerKit helpers for language code selection. |
| YoutubeExplode.Converter/ConversionRequestBuilder.cs | Moves extension usage to PowerKit. |
| YoutubeExplode.Converter/ConversionExtensions.cs | Moves extension usage to PowerKit. |
| YoutubeExplode.Converter.Tests/YoutubeExplode.Converter.Tests.csproj | Adds PowerKit for temp dirs/files and file/HTTP helpers in tests. |
| YoutubeExplode.Converter.Tests/Utils/TempFile.cs | Removes local temp file helper (replaced by PowerKit). |
| YoutubeExplode.Converter.Tests/Utils/TempDir.cs | Removes local temp dir helper (replaced by PowerKit). |
| YoutubeExplode.Converter.Tests/Utils/FFmpeg.cs | Switches helper usage to PowerKit. |
| YoutubeExplode.Converter.Tests/Utils/Extensions/HttpExtensions.cs | Removes local download helper (replaced by PowerKit). |
| YoutubeExplode.Converter.Tests/Utils/Extensions/FileExtensions.cs | Removes local file-bytes helper (replaced by PowerKit). |
| YoutubeExplode.Converter.Tests/SubtitleSpecs.cs | Switches temp dir + file content checks to PowerKit. |
| YoutubeExplode.Converter.Tests/GeneralSpecs.cs | Switches temp dir + file content checks to PowerKit. |
| YoutubeExplode.Converter.Tests/EnvironmentSpecs.cs | Switches temp dir helper to PowerKit. |
| Directory.Packages.props | Adds/updates central versions for JsonExtensions/PowerKit and bumps prerelease package versions. |
Comments suppressed due to low confidence (1)
YoutubeExplode/Bridge/SearchResponse.cs:42
- Same null-propagation issue as
Videos:ContentRoot?.EnumerateDescendantProperties(...)may benull, but.Select(...)/.ToArray()are called unconditionally. This can throw before the??fallback is reached. Add null-propagation on the LINQ chain or coalesce to an empty sequence earlier.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## prime #948 +/- ##
============================
============================
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replaces local utility/extension files across all projects with the
PowerKitandJsonExtensionsNuGet packages, and updates both packages to their latest pre-release versions. Also applies further cleanups using PowerKit's extension methods.Changes Made
Main library (
YoutubeExplode)PathExtensions.cs—Path.SanitizeFileNamereplaced byPath.EscapeFileNamefromPowerKit.ExtensionsAsyncCollectionExtensions.cs—OfTypeAsynccovered byPowerKit.Extensions.AsyncEnumerableExtensionsHttpExtensions.cs—CloneandHeadAsynccovered byPowerKit.Extensions.HttpRequestMessageExtensions/HttpClientExtensionsUriExtensions.cs—Domaincovered byPowerKit.Extensions.UriExtensionsXElementExtensions.cs—StripNamespacescovered byPowerKit.Extensions.XElementExtensionsHash.cs—Hash.Computereplaced byHashAlgorithm.ComputeHashstatic extension fromPowerKit.ExtensionsClientDelegatingHandler.cs— replaced byPowerKit.ClientDelegatingHandlerJsonExtensions.csdown to justEnumerateDescendantProperties(YouTube-specific recursive helper); all other methods replaced byJsonExtensionsNuGet package (JsonExtensions.Reading)SwapCharsintoSwapCipherOperation(only used in one place) and removed it fromStringExtensionsint/long/double/DateTimeOffset.TryParse(…, out var result) ? result : (T?)nullternaries withT.ParseOrNull(…)from PowerKit's type extensions across bridge and model files.FirstOrDefault(s => !string.IsNullOrWhiteSpace(s))with.WhereNotNullOrWhiteSpace().FirstOrDefault()inVideoWatchPage.cs.NullIfWhiteSpace()calls that followed whitespace-filteringFirstOrDefaultinVideoWatchPage.csConverter (
YoutubeExplode.Converter)AsyncCollectionExtensions.cs,GenericExtensions.cs,StringExtensions.cs— all covered byPowerKitProgressMuxer.cs— replaced byPowerKit.ProgressMuxer(now normalizes by weighted average, matching the local implementation)PowerKitpackage reference; updated usings inConverter.cs,FFmpeg.cs,ConversionRequestBuilder.cs,ConversionExtensions.cs,LanguageExtensions.csMath.Clamp(value, 0, 1)withvalue.Clamp(0, 1)using PowerKit'sComparableExtensionsinFFmpeg.csTest projects (
YoutubeExplode.Tests,YoutubeExplode.Converter.Tests)TempFile.cs/TempDir.cs— replaced byPowerKit.TempFile/PowerKit.TempDirectoryFileExtensions.csfromYoutubeExplode.Converter.Tests—File.ContainsBytesreplaced byFile.ContainsfromPowerKit.ExtensionsHttpExtensions.csfromYoutubeExplode.Converter.Tests—DownloadAsynccovered byPowerKit.Extensions.HttpClientExtensionsPowerKitpackage reference to both test projects; updated all call sitesGUI demo (
YoutubeExplode.Demo.Gui)PowerKitpackage reference; replacedPath.SanitizeFileNamewithPath.EscapeFileNameDelegateProgress.cs— replaced byPowerKit.DelegateProgress<T>Package versions
PowerKit: updated to0.0.0-a.9(latest pre-release)PolyShim: updated to2.11.0-a.1(latest pre-release)JsonExtensions:1.2.0(runtime dependency, noPrivateAssets)