📝 Add docstrings to feat/import-subtitle-from-stream#222
Merged
mkdir700 merged 1 commit intofeat/import-subtitle-from-streamfrom Oct 17, 2025
Merged
Conversation
Docstrings generation was requested by @mkdir700. * #221 (comment) The following files were modified: * `src/main/ipc.ts` * `src/renderer/src/pages/player/PlayerPage.tsx` * `src/renderer/src/pages/player/components/SubtitleListPanel.tsx`
Contributor
Author
|
Important Review skippedCodeRabbit bot authored PR detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
mkdir700
approved these changes
Oct 17, 2025
mkdir700
added a commit
that referenced
this pull request
Oct 17, 2025
* feat(media): add subtitle stream handling and extraction - Added new IPC channels for handling subtitle streams: Media_GetSubtitleStreams and Media_ExtractSubtitle - Implemented SubtitleExtractorService to manage subtitle extraction - Updated MediaParserService to include methods for retrieving subtitle stream information using ffprobe - Integrated subtitle stream handling in ipc.ts with appropriate handlers - Introduced SubtitleTrackSelector component for UI interaction with subtitle streams * feat(player): enhance subtitle import with caching - integrate SubtitleLibraryService to cache parsed subtitles - add current video context to enable subtitle persistence - log caching success and handle potential errors gracefully - ensure subtitles are stored with associated video ID * 📝 Add docstrings to `feat/import-subtitle-from-stream` (#222) Docstrings generation was requested by @mkdir700. * #221 (comment) The following files were modified: * `src/main/ipc.ts` * `src/renderer/src/pages/player/PlayerPage.tsx` * `src/renderer/src/pages/player/components/SubtitleListPanel.tsx` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * refactor: make subtitle temp cleanup async * refactor: replace blocking fs.existsSync with async fs.promises.access in SubtitleExtractorService * refactor: replace blocking fs.existsSync with async fs.promises in critical paths - Replace fs.existsSync with fs.promises.access in IPC handler - Optimize MediaParserService by merging existsSync + statSync into single stat call - Convert FFmpegService.fastCheckFFmpegExists to async method - Replace all blocking file checks in async methods across services This eliminates event loop blocking in hot paths and improves I/O performance. * fix: add context object to subtitle detection logger for searchability Add path and videoId to logger.info when no subtitle streams detected, improving log searchability and debugging capabilities. * refactor: consolidate path conversion logic to use PathConverter - Remove duplicate convertFileUrlToLocalPath from MediaParserService - Remove duplicate convertFileUrlToLocalPath from FFmpegService - Remove convertFileUrlToLocalPathAsync from ParallelVideoProcessor - Replace all calls with PathConverter.convertToLocalPath for consistency - Add proper error handling for path conversion failures - Remove ~100+ lines of duplicate code * refactor(MediaParser): fix ffprobe service usage and prevent double settle - Use this.ffmpegService.getFFprobePath() instead of inline new FFmpegService() - Add -select_streams 's' to only return subtitle streams for better performance - Implement settled flag to prevent double settle race conditions - Change timeout handler to resolve(null) instead of reject for consistency - Add settled checks in all handlers (timeout/close/error) before resolving/rejecting - Ensure ffprobe process is killed on timeout with proper logging context * refactor(player): replace hardcoded Chinese text with i18n in subtitle empty state - Replace hardcoded Chinese text in SubtitleListPanel empty state options - Update embedded subtitle option: title, description, and action button - Update external subtitle option: title and description - Update AI-generated subtitle option: title, description, and action button - All i18n keys already exist in language packs (zh-cn, zh-tw, en-us, ja-jp, ru-ru) * feat(SubtitleExtractor): implement temporary subtitle file cleanup functionality - Add new IPC channel `subtitle-extractor:cleanup-temp` for cleaning up temporary subtitle files. - Implement `cleanupTempFiles` method in `SubtitleExtractorService` to remove subtitle files matching the pattern from the system's temporary directory. - Enhance logging for cleanup operations, including success and error messages. - Add unit tests for `cleanupTempFiles` to ensure correct functionality and logging behavior. * docs: update CLAUDE.md with testing and resource management guidelines - Add important notes on mocking `node:fs` and `node:fs/promises` in the global test setup. - Include examples for unmocking these modules in test files for real filesystem operations. - Introduce resource management and cleanup section detailing temporary file cleanup practices. - Outline best practices for implementing `cleanupTempFiles()` and IPC channel usage for manual cleanup triggers. - Specify naming conventions for temporary files to facilitate pattern matching and cleanup. * fix(SubtitleExtractorService): refactor cleanupTempFiles to use async/await - Update cleanupTempFiles method to be asynchronous, ensuring non-blocking file operations. - Replace synchronous file reading and deletion with their asynchronous counterparts to prevent event loop blocking. - Enhance error handling during file deletion and log the number of cleaned files. - Update related IPC handlers and unit tests to accommodate the async changes, ensuring proper functionality and logging behavior. * test(FFmpegService): refactor integration tests to use async/await for filesystem checks - Update FFmpegService integration tests to utilize async/await for filesystem operations, ensuring non-blocking behavior. - Mock fs.promises.stat to simulate various scenarios, including existing files, non-existent files, and directories. - Enhance error handling in tests to gracefully manage filesystem errors. - Ensure all tests correctly await the results of fastCheckFFmpegExists, improving test reliability and performance. * refactor(SubtitleExtractorService): enhance temporary subtitle filename generation and update unit tests - Modify the random string generation in generateTempSubtitlePath to ensure it only contains lowercase letters and numbers. - Update unit tests to improve validation of log messages and regex patterns for subtitle filenames. - Remove redundant test cases and streamline the cleanupTempFile tests for better clarity and efficiency. * refactor(ipc): enhance cleanupTemp IPC handler with logging and return count - Update the cleanupTemp IPC handler to log the number of cleaned temporary subtitle files. - Refactor the useSubtitleOverlay hook to remove unnecessary memoization and improve clarity by directly using currentSubtitleData. - Ensure consistent handling of subtitle data across the component, enhancing performance and readability. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
mkdir700
added a commit
that referenced
this pull request
Oct 17, 2025
* feat(media): add subtitle stream handling and extraction - Added new IPC channels for handling subtitle streams: Media_GetSubtitleStreams and Media_ExtractSubtitle - Implemented SubtitleExtractorService to manage subtitle extraction - Updated MediaParserService to include methods for retrieving subtitle stream information using ffprobe - Integrated subtitle stream handling in ipc.ts with appropriate handlers - Introduced SubtitleTrackSelector component for UI interaction with subtitle streams * feat(player): enhance subtitle import with caching - integrate SubtitleLibraryService to cache parsed subtitles - add current video context to enable subtitle persistence - log caching success and handle potential errors gracefully - ensure subtitles are stored with associated video ID * 📝 Add docstrings to `feat/import-subtitle-from-stream` (#222) Docstrings generation was requested by @mkdir700. * #221 (comment) The following files were modified: * `src/main/ipc.ts` * `src/renderer/src/pages/player/PlayerPage.tsx` * `src/renderer/src/pages/player/components/SubtitleListPanel.tsx` Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * refactor: make subtitle temp cleanup async * refactor: replace blocking fs.existsSync with async fs.promises.access in SubtitleExtractorService * refactor: replace blocking fs.existsSync with async fs.promises in critical paths - Replace fs.existsSync with fs.promises.access in IPC handler - Optimize MediaParserService by merging existsSync + statSync into single stat call - Convert FFmpegService.fastCheckFFmpegExists to async method - Replace all blocking file checks in async methods across services This eliminates event loop blocking in hot paths and improves I/O performance. * fix: add context object to subtitle detection logger for searchability Add path and videoId to logger.info when no subtitle streams detected, improving log searchability and debugging capabilities. * refactor: consolidate path conversion logic to use PathConverter - Remove duplicate convertFileUrlToLocalPath from MediaParserService - Remove duplicate convertFileUrlToLocalPath from FFmpegService - Remove convertFileUrlToLocalPathAsync from ParallelVideoProcessor - Replace all calls with PathConverter.convertToLocalPath for consistency - Add proper error handling for path conversion failures - Remove ~100+ lines of duplicate code * refactor(MediaParser): fix ffprobe service usage and prevent double settle - Use this.ffmpegService.getFFprobePath() instead of inline new FFmpegService() - Add -select_streams 's' to only return subtitle streams for better performance - Implement settled flag to prevent double settle race conditions - Change timeout handler to resolve(null) instead of reject for consistency - Add settled checks in all handlers (timeout/close/error) before resolving/rejecting - Ensure ffprobe process is killed on timeout with proper logging context * refactor(player): replace hardcoded Chinese text with i18n in subtitle empty state - Replace hardcoded Chinese text in SubtitleListPanel empty state options - Update embedded subtitle option: title, description, and action button - Update external subtitle option: title and description - Update AI-generated subtitle option: title, description, and action button - All i18n keys already exist in language packs (zh-cn, zh-tw, en-us, ja-jp, ru-ru) * feat(SubtitleExtractor): implement temporary subtitle file cleanup functionality - Add new IPC channel `subtitle-extractor:cleanup-temp` for cleaning up temporary subtitle files. - Implement `cleanupTempFiles` method in `SubtitleExtractorService` to remove subtitle files matching the pattern from the system's temporary directory. - Enhance logging for cleanup operations, including success and error messages. - Add unit tests for `cleanupTempFiles` to ensure correct functionality and logging behavior. * docs: update CLAUDE.md with testing and resource management guidelines - Add important notes on mocking `node:fs` and `node:fs/promises` in the global test setup. - Include examples for unmocking these modules in test files for real filesystem operations. - Introduce resource management and cleanup section detailing temporary file cleanup practices. - Outline best practices for implementing `cleanupTempFiles()` and IPC channel usage for manual cleanup triggers. - Specify naming conventions for temporary files to facilitate pattern matching and cleanup. * fix(SubtitleExtractorService): refactor cleanupTempFiles to use async/await - Update cleanupTempFiles method to be asynchronous, ensuring non-blocking file operations. - Replace synchronous file reading and deletion with their asynchronous counterparts to prevent event loop blocking. - Enhance error handling during file deletion and log the number of cleaned files. - Update related IPC handlers and unit tests to accommodate the async changes, ensuring proper functionality and logging behavior. * test(FFmpegService): refactor integration tests to use async/await for filesystem checks - Update FFmpegService integration tests to utilize async/await for filesystem operations, ensuring non-blocking behavior. - Mock fs.promises.stat to simulate various scenarios, including existing files, non-existent files, and directories. - Enhance error handling in tests to gracefully manage filesystem errors. - Ensure all tests correctly await the results of fastCheckFFmpegExists, improving test reliability and performance. * refactor(SubtitleExtractorService): enhance temporary subtitle filename generation and update unit tests - Modify the random string generation in generateTempSubtitlePath to ensure it only contains lowercase letters and numbers. - Update unit tests to improve validation of log messages and regex patterns for subtitle filenames. - Remove redundant test cases and streamline the cleanupTempFile tests for better clarity and efficiency. * refactor(ipc): enhance cleanupTemp IPC handler with logging and return count - Update the cleanupTemp IPC handler to log the number of cleaned temporary subtitle files. - Refactor the useSubtitleOverlay hook to remove unnecessary memoization and improve clarity by directly using currentSubtitleData. - Ensure consistent handling of subtitle data across the component, enhancing performance and readability. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Docstrings generation was requested by @mkdir700.
The following files were modified:
src/main/ipc.tssrc/renderer/src/pages/player/PlayerPage.tsxsrc/renderer/src/pages/player/components/SubtitleListPanel.tsxThese file types are not supported
src/renderer/src/i18n/locales/en-us.jsonsrc/renderer/src/i18n/locales/ja-jp.jsonsrc/renderer/src/i18n/locales/ru-ru.jsonsrc/renderer/src/i18n/locales/zh-cn.jsonsrc/renderer/src/i18n/locales/zh-tw.jsonℹ️ Note