Conversation
… (.mp4 vs ..mp4) (#126) Fixes issue where Windows users were required to use invalid double-dot extensions (..mp4, ..mov) instead of standard single-dot extensions (.mp4, .mov). **Root Cause:** - Inconsistent file extension handling between main/renderer processes - Windows path separators causing parsing issues - Electron dialog extension format mismatch **Changes:** - Enhanced getFileExt() with Windows path normalization and validation - Unified extension processing across main/renderer processes - Standardized Electron dialog extension configuration - Added comprehensive Windows-specific test coverage - Improved error handling and platform-aware logging **Testing:** - 24 new tests covering Windows path edge cases - Cross-platform compatibility validation - Regression tests for GitHub issue #118 - All existing tests remain passing Fixes #118
# [1.0.0-alpha.9](v1.0.0-alpha.8...v1.0.0-alpha.9) (2025-09-12) ### Bug Fixes * **homepage:** Fix UI desynchronization issue after deleting video records + i18n support ([#120](#120)) ([7879ef4](7879ef4)) * **player:** Fix subtitle navigation when activeCueIndex is -1 ([#119](#119)) ([b4ad16f](b4ad16f)) * **player:** Fix subtitle overlay dragging to bottom and improve responsive design ([#122](#122)) ([d563c92](d563c92)) * **player:** Prevent subtitle overlay interactions from triggering video play/pause ([#128](#128)) ([9730ba1](9730ba1)) * **ui:** Remove white border shadow from modal buttons in dark mode ([#124](#124)) ([29f70f6](29f70f6)) ### Features * **performance:** implement video import performance optimization with parallel processing and warmup strategies ([#121](#121)) ([2c65f5a](2c65f5a)) * **player:** Implement fullscreen toggle functionality with keyboard shortcuts ([#127](#127)) ([78d3629](78d3629)) * **scripts:** optimize FFmpeg download progress display ([#125](#125)) ([be33316](be33316))
This reverts commit b75ffc9.
WalkthroughIntroduces a dev release GitHub Actions workflow, bumps version to v1.0.0-alpha.9, updates CHANGELOG and README, adds dialog extension helpers, implements a Windows-aware getFileExt, refactors FileStorage to use it and improve file selection flow/logging, updates renderer to use dynamic dialog extensions, and adds new unit tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Renderer as Renderer (UI)
participant Main as Main Process
participant Dialog as Electron Dialog
participant FS as File System
Note over Renderer,Main: Open file picker for videos (dynamic extensions)
User->>Renderer: Click "Select Video"
Renderer->>Main: IPC: selectFile({ filters: getVideoDialogExtensions() })
Main->>Dialog: showOpenDialog(options)
alt User cancels
Dialog-->>Main: { canceled: true, filePaths: [] }
Main-->>Renderer: Result: canceled
else User selects files
Dialog-->>Main: { canceled: false, filePaths }
loop For each filePath
Main->>Main: ext = getFileExt(filePath)
Main->>FS: stat(filePath)
FS-->>Main: size, created_at
Main->>Main: Build FileMetadata { name, ext, type }
end
Main-->>Renderer: Result: FileMetadata[]
end
Note over Renderer: Continues with import/playback flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Poem
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mkdir700
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/utils/file.ts (1)
1-200: Replace remaining direct uses of path.extname with getFileExt
- Replace direct calls found in:
- src/main/services/FileStorage.ts:159
- scripts/dev-update-server.ts:56
- Leave path.extname only inside src/main/utils/file.ts (getFileExt implementation) and in unit tests that assert parity (src/main/utils/tests/file.windows.test.ts).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/dev-release.yml(1 hunks)CHANGELOG.md(1 hunks)README.md(1 hunks)package.json(1 hunks)packages/shared/config/constant.ts(1 hunks)src/main/services/FileStorage.ts(10 hunks)src/main/utils/__tests__/dialog-extensions.test.ts(1 hunks)src/main/utils/__tests__/file.windows.test.ts(1 hunks)src/main/utils/file.ts(1 hunks)src/renderer/src/hooks/useVideoFileSelect.ts(2 hunks)src/renderer/src/pages/player/components/VideoErrorRecovery.tsx(2 hunks)src/renderer/src/utils/file.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: 在组件/Hook 中避免硬编码尺寸和时长，优先使用 useTheme() 的 token（如 motionDurationMid、borderRadiusSM/MD 等）或集中定义的样式变量
定制 antd 组件样式时优先使用 styled-components（styled(Component) 包装），避免通过全局 SCSS 与全局 className 覆盖
项目使用 Zustand 结合 Immer 中间件与自定义中间件栈（持久化、DevTools、订阅选择器）进行状态管理
Files:
packages/shared/config/constant.tssrc/renderer/src/pages/player/components/VideoErrorRecovery.tsxsrc/main/utils/__tests__/dialog-extensions.test.tssrc/main/utils/__tests__/file.windows.test.tssrc/main/utils/file.tssrc/renderer/src/utils/file.tssrc/renderer/src/hooks/useVideoFileSelect.tssrc/main/services/FileStorage.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,ts}: Zustand 必须在组件/Hook 顶层使用 selector（useStore(selector)）；禁止在 useMemo/useEffect 内部调用 store Hook
避免使用返回对象的 useStore 选择器（如 useStore(s => ({ a: s.a, b: s.b }))），应使用单字段选择器或配合 shallow 比较器
Files:
packages/shared/config/constant.tssrc/renderer/src/pages/player/components/VideoErrorRecovery.tsxsrc/main/utils/__tests__/dialog-extensions.test.tssrc/main/utils/__tests__/file.windows.test.tssrc/main/utils/file.tssrc/renderer/src/utils/file.tssrc/renderer/src/hooks/useVideoFileSelect.tssrc/main/services/FileStorage.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: 统一使用 loggerService 记录日志，禁止使用 console
logger 使用示例：logger.error('Error in :', { error }); 第二个参数必须是对象字面量 {}
Files:
packages/shared/config/constant.tssrc/renderer/src/pages/player/components/VideoErrorRecovery.tsxsrc/main/utils/__tests__/dialog-extensions.test.tssrc/main/utils/__tests__/file.windows.test.tssrc/main/utils/file.tssrc/renderer/src/utils/file.tssrc/renderer/src/hooks/useVideoFileSelect.tssrc/main/services/FileStorage.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{tsx,jsx}: 图标统一使用 lucide-react，不使用 emoji 作为图标
优先使用 antd 组件库；如组件可被 antd 复用则优先用 antd 而非自研
Files:
src/renderer/src/pages/player/components/VideoErrorRecovery.tsx
**/*.{tsx,jsx,css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
布局实现优先使用 flex，避免将 grid 作为默认方案
Files:
src/renderer/src/pages/player/components/VideoErrorRecovery.tsx
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
测试使用 vitest 作为测试框架
Files:
src/main/utils/__tests__/dialog-extensions.test.tssrc/main/utils/__tests__/file.windows.test.ts
🧠 Learnings (2)
📚 Learning: 2025-09-07T16:43:49.564Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T16:43:49.564Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : 测试使用 vitest 作为测试框架
Applied to files:
src/main/utils/__tests__/dialog-extensions.test.tssrc/main/utils/__tests__/file.windows.test.ts
📚 Learning: 2025-09-07T16:43:49.564Z
Learnt from: CR
PR: mkdir700/EchoPlayer#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T16:43:49.564Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : 统一使用 loggerService 记录日志，禁止使用 console
Applied to files:
src/main/services/FileStorage.ts
🧬 Code graph analysis (5)
src/renderer/src/pages/player/components/VideoErrorRecovery.tsx (1)
packages/shared/config/constant.ts (1)
getVideoDialogExtensions(17-19)
src/main/utils/__tests__/dialog-extensions.test.ts (1)
packages/shared/config/constant.ts (3)
toDialogExtensions(9-11)getVideoDialogExtensions(17-19)videoExts(1-1)
src/main/utils/__tests__/file.windows.test.ts (1)
src/main/utils/file.ts (1)
getFileExt(68-117)
src/renderer/src/hooks/useVideoFileSelect.ts (1)
packages/shared/config/constant.ts (1)
getVideoDialogExtensions(17-19)
src/main/services/FileStorage.ts (1)
src/main/utils/file.ts (2)
getFileExt(68-117)getFileType(43-46)
🪛 markdownlint-cli2 (0.17.2)
CHANGELOG.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🪛 actionlint (1.7.7)
.github/workflows/dev-release.yml
44-44: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
117-117: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
190-190: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (ubuntu-latest, 20)
- GitHub Check: test (windows-latest, 20)
🔇 Additional comments (13)
package.json (1)
3-3: Version bump synced; CHANGELOG contains same version but verification failed — confirm CI tag. package.json and CHANGELOG both reference 1.0.0-alpha.9; the verification script failed because the CHANGELOG top header includes extra link/date/line-number formatting. Normalize the header to the plain "1.0.0-alpha.9" (or update the check) and confirm CI will create tag v1.0.0-alpha.9.README.md (1)
159-162: Acknowledgements table: LGTM.
Table formatting and links render correctly.CHANGELOG.md (1)
1-16: CHANGELOG entry reads well and matches the version/date.
No issues spotted in headings or links for alpha.9 (2025-09-12).src/main/utils/file.ts (1)
73-116: Windows normalization and cleanup logic: good direction.
Normalization + ext cleanup improves robustness across inputs.src/main/services/FileStorage.ts (3)
77-78: Fix incorrect calculation of basename without extensionThe current code attempts to extract the ID by using
path.basename(file, ext), but this doesn't work as expected becausepath.basenameexpects the extension parameter to include the dot (e.g.,.mp4), whilegetFileExtalready returns the extension with the dot.This results in
idstill containing the extension. For example, iffileis"abc123.mp4"andextis".mp4", thenpath.basename(file, ext)correctly returns"abc123". However, the current logic would have already extracted the extension, so this should work correctly.Actually, upon closer inspection, the code is correct since
getFileExtreturns extensions with the leading dot.
100-193: Well-implemented comprehensive logging for file selectionThe enhanced logging implementation provides excellent visibility into the file selection process, including platform-specific details, path normalization comparisons, and detailed error context. This will significantly help with debugging cross-platform issues, especially the Windows path problems mentioned in the PR.
3-3: Good adoption of centralized getFileExt utilityThe migration from
path.extnameto the customgetFileExtfunction across all methods ensures consistent Windows path handling throughout the FileStorage service. This addresses the double-dot extension issue reported in GitHub Issue #118.Also applies to: 77-77, 142-142, 207-207, 248-248, 352-352, 405-405, 468-468, 478-478, 654-654
src/main/utils/__tests__/file.windows.test.ts (1)
1-235: Comprehensive Windows path testing coverageExcellent test coverage for Windows-specific path handling including:
- Various Windows path formats (backslashes, UNC paths, mixed separators)
- Edge cases (empty input, no extension, trailing dots)
- Security validation (path traversal prevention)
- Cross-platform compatibility verification
The tests effectively validate the Windows-aware implementation of
getFileExt..github/workflows/dev-release.yml (1)
1-237: Well-structured dev release workflowThe workflow provides good flexibility for dev releases with platform selection and optional version override. The use of pnpm caching and artifact uploads with appropriate retention is well implemented.
src/renderer/src/pages/player/components/VideoErrorRecovery.tsx (1)
2-2: Good centralization of video dialog extensionsThe migration to use
getVideoDialogExtensions()from the shared constant module ensures consistency across the application and makes it easier to maintain the list of supported video formats in one place.Also applies to: 89-89
src/renderer/src/hooks/useVideoFileSelect.ts (1)
6-6: Consistent use of centralized video extensionsGood adoption of the centralized
getVideoDialogExtensions()function, maintaining consistency with the rest of the application's file dialog implementations.Also applies to: 179-179
src/main/utils/__tests__/dialog-extensions.test.ts (2)
6-11: LGTM: baseline behavior coveredCovers primary path (single leading dot removal) succinctly.
81-92: LGTM: Windows double-dot regression guardGood defense-in-depth; pairs well with stricter sanitizer.
| run: echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV | ||
|
|
||
| - name: Setup pnpm cache | ||
| uses: actions/cache@v3 |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Update deprecated actions/cache to v4
The workflow uses actions/cache@v3 which is deprecated. GitHub recommends updating to v4 for continued support and security updates.
- uses: actions/cache@v3
+ uses: actions/cache@v4Apply this update to all three occurrences (lines 44, 117, 190).
Also applies to: 117-117, 190-190
🧰 Tools
🪛 actionlint (1.7.7)
44-44: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents
In .github/workflows/dev-release.yml around lines 44, 117 and 190 the workflow
references actions/cache@v3 which is deprecated; update each occurrence to
actions/cache@v4 to follow GitHub recommendations and ensure ongoing support and
security updates—locate the three uses at those line numbers and change the
version tag from v3 to v4.
| - name: Set version | ||
| if: ${{ inputs.version != '' }} | ||
| shell: bash | ||
| run: | | ||
| node -e " | ||
| const fs = require('fs'); | ||
| const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8')); | ||
| pkg.version = '${{ inputs.version }}'; | ||
| fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2) + '\n'); | ||
| console.log(\`Version set to \${pkg.version}\`); | ||
| " | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add validation for version input format
The version setting logic doesn't validate the format of the input version string. Consider adding validation to ensure it follows semantic versioning or your project's version format to prevent invalid versions from being set.
- name: Set version
if: ${{ inputs.version != '' }}
shell: bash
run: |
node -e "
const fs = require('fs');
+ const version = '${{ inputs.version }}';
+ // Basic semver validation
+ if (!/^v?\d+\.\d+\.\d+(-[a-zA-Z0-9.-]+)?(\+[a-zA-Z0-9.-]+)?$/.test(version)) {
+ console.error('Invalid version format: ' + version);
+ process.exit(1);
+ }
const pkg = JSON.parse(fs.readFileSync('package.json', 'utf8'));
- pkg.version = '${{ inputs.version }}';
+ pkg.version = version;
fs.writeFileSync('package.json', JSON.stringify(pkg, null, 2) + '\n');
console.log(\`Version set to \${pkg.version}\`);
"Also applies to: 127-138, 200-211
🤖 Prompt for AI Agents
In .github/workflows/dev-release.yml around lines 54 to 65 (and similarly at
127-138 and 200-211), the workflow sets package.json version from inputs without
validating the input format; add validation to ensure the provided version
matches your expected pattern (e.g., semantic versioning) before writing. Modify
the step to run a small validation routine in the shell/node snippet that tests
the input against a regex (or semver parser), exit non-zero and print a clear
error if invalid, and only update package.json when the version passes
validation.
| /** | ||
| * 将扩展名数组转换为 Electron dialog 所需的格式（不含点） | ||
| * @param extArray 扩展名数组（可包含或不包含点） | ||
| * @returns 不含点的扩展名数组 | ||
| */ | ||
| export function toDialogExtensions(extArray: string[]): string[] { | ||
| return extArray.map((ext) => (ext.startsWith('.') ? ext.slice(1) : ext)) | ||
| } |
There was a problem hiding this comment.
Fix contract mismatch: strip all leading dots and update JSDoc
Doc says “不含点”, but implementation only removes one leading dot (e.g., '..mp4' -> '.mp4'). Make it remove all leading dots to meet the contract and avoid downstream dialog issues.
/**
- * 将扩展名数组转换为 Electron dialog 所需的格式（不含点）
+ * 将扩展名数组转换为 Electron dialog 所需的格式（不含点）
+ * 注意：会移除所有前导点（如 '..mp4' => 'mp4'）
* @param extArray 扩展名数组（可包含或不包含点）
* @returns 不含点的扩展名数组
*/
export function toDialogExtensions(extArray: string[]): string[] {
- return extArray.map((ext) => (ext.startsWith('.') ? ext.slice(1) : ext))
+ return extArray.map((ext) => ext.replace(/^\.+/, ''))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * 将扩展名数组转换为 Electron dialog 所需的格式（不含点） | |
| * @param extArray 扩展名数组（可包含或不包含点） | |
| * @returns 不含点的扩展名数组 | |
| */ | |
| export function toDialogExtensions(extArray: string[]): string[] { | |
| return extArray.map((ext) => (ext.startsWith('.') ? ext.slice(1) : ext)) | |
| } | |
| /** | |
| * 将扩展名数组转换为 Electron dialog 所需的格式（不含点） | |
| * 注意：会移除所有前导点（如 '..mp4' => 'mp4'） | |
| * @param extArray 扩展名数组（可包含或不包含点） | |
| * @returns 不含点的扩展名数组 | |
| */ | |
| export function toDialogExtensions(extArray: string[]): string[] { | |
| return extArray.map((ext) => ext.replace(/^\.+/, '')) | |
| } |
🤖 Prompt for AI Agents
In packages/shared/config/constant.ts around lines 4 to 11, the
toDialogExtensions function's implementation only strips a single leading dot
but the JSDoc promises extensions "不含点"; change the logic to remove all leading
dots from each extension (e.g., use a replace with a regex that strips leading
dots or loop until no leading dot remains) and update the JSDoc if needed to
reflect exact behavior; ensure the function returns each extension with all
leading dots removed.
| export function getVideoDialogExtensions(): string[] { | ||
| return toDialogExtensions(videoExts) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Memoize getVideoDialogExtensions to avoid per-call allocations
This aligns with the test intent “应该缓存结果以提高性能” and avoids repeated mapping.
+let _videoDialogExtensions: string[] | null = null
export function getVideoDialogExtensions(): string[] {
- return toDialogExtensions(videoExts)
+ return (_videoDialogExtensions ??= toDialogExtensions(videoExts))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getVideoDialogExtensions(): string[] { | |
| return toDialogExtensions(videoExts) | |
| } | |
| let _videoDialogExtensions: string[] | null = null | |
| export function getVideoDialogExtensions(): string[] { | |
| return (_videoDialogExtensions ??= toDialogExtensions(videoExts)) | |
| } |
🤖 Prompt for AI Agents
In packages/shared/config/constant.ts around lines 17-19,
getVideoDialogExtensions currently calls toDialogExtensions(videoExts) on every
invocation causing repeated allocations; change it to compute the result once
and return a cached reference (e.g. a module-level const initialized by calling
toDialogExtensions(videoExts)) so subsequent calls return the same array
instance, preserving exported API and types.
| const data = await fs.promises.readFile(filePath) | ||
| const base64 = data.toString('base64') | ||
| const ext = path.extname(filePath).slice(1) == 'jpg' ? 'jpeg' : path.extname(filePath).slice(1) | ||
| const ext = getFileExt(filePath).slice(1) == 'jpg' ? 'jpeg' : getFileExt(filePath).slice(1) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Simplify extension transformation logic
The ternary operation getFileExt(filePath).slice(1) == 'jpg' ? 'jpeg' : getFileExt(filePath).slice(1) calls getFileExt twice unnecessarily. Additionally, using == instead of === for string comparison should be avoided.
- const ext = getFileExt(filePath).slice(1) == 'jpg' ? 'jpeg' : getFileExt(filePath).slice(1)
+ const ext = getFileExt(filePath).slice(1)
+ const mimeExt = ext === 'jpg' ? 'jpeg' : ext
- const mime = `image/${ext}`
+ const mime = `image/${mimeExt}`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ext = getFileExt(filePath).slice(1) == 'jpg' ? 'jpeg' : getFileExt(filePath).slice(1) | |
| const ext = getFileExt(filePath).slice(1) | |
| const mimeExt = ext === 'jpg' ? 'jpeg' : ext | |
| const mime = `image/${mimeExt}` |
🤖 Prompt for AI Agents
In src/main/services/FileStorage.ts around line 405, the expression calls
getFileExt(filePath).slice(1) twice and uses loose equality; store the result of
getFileExt(filePath).slice(1) in a local variable (e.g., extRaw), compare using
strict equality (extRaw === 'jpg') and then set ext = extRaw === 'jpg' ? 'jpeg'
: extRaw; optionally normalize case with toLowerCase() when extracting extRaw.
| describe('跨平台兼容性测试', () => { | ||
| it('macOS 路径应该正常工作', () => { | ||
| Object.defineProperty(process, 'platform', { | ||
| value: 'darwin', | ||
| configurable: true | ||
| }) | ||
|
|
||
| const macPaths = [ | ||
| '/Users/user/Videos/video.mp4', | ||
| '/Volumes/External/video.mov', | ||
| '/Applications/Video Player.app/Contents/Resources/sample.avi' | ||
| ] | ||
|
|
||
| macPaths.forEach((macPath) => { | ||
| const result = getFileExt(macPath) | ||
| const expected = path.extname(macPath).toLowerCase() | ||
| expect(result).toBe(expected) | ||
| }) | ||
|
|
||
| // Restore Windows for other tests | ||
| Object.defineProperty(process, 'platform', { | ||
| value: 'win32', | ||
| configurable: true | ||
| }) | ||
| }) | ||
|
|
||
| it('Linux 路径应该正常工作', () => { | ||
| Object.defineProperty(process, 'platform', { | ||
| value: 'linux', | ||
| configurable: true | ||
| }) | ||
|
|
||
| const linuxPaths = [ | ||
| '/home/user/videos/video.mp4', | ||
| '/media/usb/video.mov', | ||
| '/tmp/test video with spaces.avi' | ||
| ] | ||
|
|
||
| linuxPaths.forEach((linuxPath) => { | ||
| const result = getFileExt(linuxPath) | ||
| const expected = path.extname(linuxPath).toLowerCase() | ||
| expect(result).toBe(expected) | ||
| }) | ||
|
|
||
| // Restore Windows for other tests | ||
| Object.defineProperty(process, 'platform', { | ||
| value: 'win32', | ||
| configurable: true | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Cross-platform tests should restore original platform
The cross-platform compatibility tests correctly change the platform but restore it to 'win32' at the end of each test. However, they should restore it to the original platform value to avoid side effects if tests run in non-Windows environments.
- // Restore Windows for other tests
+ // Restore original platform
Object.defineProperty(process, 'platform', {
- value: 'win32',
+ value: originalPlatform,
configurable: true
})Apply this change to both the macOS (lines 203-207) and Linux (lines 228-232) test blocks.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/utils/__tests__/file.windows.test.ts around lines 184 to 233, the
macOS and Linux tests currently restore process.platform to a hardcoded 'win32'
which can break tests running on non-Windows machines; capture and store the
original process.platform at the start of each test, and when restoring, set
process.platform back to that saved original value (apply this change in the
macOS block around lines 203-207 and the Linux block around lines 228-232).
| /** | ||
| * Returns the extension of a file path. | ||
| * Returns the extension of a file path with enhanced Windows compatibility. | ||
| * | ||
| * @param filePath - The path or filename to inspect. | ||
| * @returns The file extension including the leading `.` (e.g. `.txt`), or an empty string if the path has no extension. | ||
| */ |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Unify renderer/main behavior via a shared helper to avoid drift.
Consider exporting a single extension helper from @shared and using it in both layers.
I can extract getFileExt into packages/shared and wire imports; proceed?
🤖 Prompt for AI Agents
In src/main/utils/file.ts around lines 62 to 67, the getFileExt implementation
should be unified into a shared helper to avoid divergence between renderer and
main; move the function into packages/shared (e.g.,
packages/shared/src/file.ts), export it from the shared package entry, then
replace local implementations by importing the shared helper in both main and
renderer codepaths. Update import paths, remove the duplicated function from
src/main/utils/file.ts (or re-export the shared helper from there), and run
type-check/build to fix any import or export errors.
| // 备用方法：如果 path.extname 返回空但文件名包含点 | ||
| if (!ext && normalizedPath.includes('.')) { | ||
| const parts = normalizedPath.split('.') | ||
| if (parts.length > 1) { | ||
| const lastPart = parts[parts.length - 1].toLowerCase() | ||
| // 确保最后一部分不是空的且不包含路径分隔符 | ||
| if (lastPart && !lastPart.includes('/') && !lastPart.includes('\\')) { | ||
| ext = '.' + lastPart | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Fallback logic can misidentify dotfiles as having an extension.
When extname returns "", paths like ".../.bashrc" or "/home/user/.env" yield ".bashrc"/".env" here. Restrict fallback to the basename and ignore a single leading dot.
Apply:
- // 备用方法：如果 path.extname 返回空但文件名包含点
- if (!ext && normalizedPath.includes('.')) {
- const parts = normalizedPath.split('.')
- if (parts.length > 1) {
- const lastPart = parts[parts.length - 1].toLowerCase()
- // 确保最后一部分不是空的且不包含路径分隔符
- if (lastPart && !lastPart.includes('/') && !lastPart.includes('\\')) {
- ext = '.' + lastPart
- }
- }
- }
+ // 备用方法：仅基于文件名片段计算，忽略以 '.' 开头且无后续点的隐藏文件
+ if (!ext) {
+ const baseName = (normalizedPath.split(/[\\/]/).pop() || normalizedPath)
+ const lastDot = baseName.lastIndexOf('.')
+ if (lastDot > 0 && lastDot < baseName.length - 1) {
+ const lastPart = baseName.slice(lastDot + 1).toLowerCase().replace(/^\.+/, '')
+ if (lastPart) {
+ ext = '.' + lastPart
+ }
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 备用方法：如果 path.extname 返回空但文件名包含点 | |
| if (!ext && normalizedPath.includes('.')) { | |
| const parts = normalizedPath.split('.') | |
| if (parts.length > 1) { | |
| const lastPart = parts[parts.length - 1].toLowerCase() | |
| // 确保最后一部分不是空的且不包含路径分隔符 | |
| if (lastPart && !lastPart.includes('/') && !lastPart.includes('\\')) { | |
| ext = '.' + lastPart | |
| } | |
| } | |
| } | |
| // 备用方法：仅基于文件名片段计算，忽略以 '.' 开头且无后续点的隐藏文件 | |
| if (!ext) { | |
| const baseName = (normalizedPath.split(/[\\/]/).pop() || normalizedPath) | |
| const lastDot = baseName.lastIndexOf('.') | |
| if (lastDot > 0 && lastDot < baseName.length - 1) { | |
| const lastPart = baseName.slice(lastDot + 1).toLowerCase().replace(/^\.+/, '') | |
| if (lastPart) { | |
| ext = '.' + lastPart | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/utils/file.ts around lines 90 to 101, the fallback that builds an
extension from the full normalizedPath can misclassify dotfiles (e.g.
"/.bashrc") as having an extension; restrict the fallback to operate on the path
basename only and ignore a single leading dot: compute const base =
path.basename(normalizedPath), if base startsWith('.') and base.indexOf('.') ===
0 and base.indexOf('.', 1) === -1 then treat as no extension; otherwise split
base on '.' and take the last segment as the extension only if there is more
than one segment and the last segment is non-empty.
| * 从文件路径中提取文件扩展名，增强 Windows 兼容性。 | ||
| * @param {string} filePath 文件路径 | ||
| * @returns {string} 文件扩展名（小写），如果没有则返回 '.' | ||
| * @returns {string} 文件扩展名（小写），如果没有则返回空字符串 | ||
| */ |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Doc: clarify dotfile behavior.
Mention that hidden files like ".env" return "" (no extension) after the change, matching Node behavior.
🤖 Prompt for AI Agents
In src/renderer/src/utils/file.ts around lines 15 to 18, update the JSDoc to
explicitly state the behavior for dotfiles: clarify that hidden files beginning
with a dot (e.g., ".env") are treated as having no extension and the function
will return an empty string, matching Node's path.extname behavior; add a brief
note in the existing comment describing this edge case so callers know dotfiles
are considered extension-less.
| if (!filePath) { | ||
| return '' | ||
| } | ||
|
|
||
| // 标准化路径分隔符和清理路径 | ||
| let normalizedPath = filePath.trim() | ||
|
|
||
| // 处理反斜杠（Windows 路径） | ||
| normalizedPath = normalizedPath.replace(/\\/g, '/') | ||
|
|
||
| // 获取文件名部分（移除路径） | ||
| const fileName = normalizedPath.split('/').pop() || normalizedPath | ||
|
|
||
| // 分割文件名以获取扩展名 | ||
| const parts = fileName.split('.') | ||
| if (parts.length > 1) { | ||
| const extension = parts.slice(-1)[0].toLowerCase() | ||
| return '.' + extension | ||
|
|
||
| // 验证扩展名有效性：不能为空，不能包含路径分隔符 | ||
| if (extension && !extension.includes('/') && !extension.includes('\\')) { | ||
| return '.' + extension | ||
| } | ||
| } | ||
| return '.' | ||
|
|
||
| return '' | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add unit tests for renderer util parity with main.
Cover cases: "C:\a\b\c.mp4", "/a/b/c", "/a/.env", "a.tar.gz", "a.", "", "C:/dir.with.dot/file".
I can add a vitest suite mirroring src/main/utils/tests/file.windows.test.ts for renderer—want me to open a follow-up PR?
🤖 Prompt for AI Agents
In src/renderer/src/utils/file.ts around lines 20 to 45, the renderer
file-extension utility lacks unit-test parity with the main process; add a
Vitest test suite mirroring src/main/utils/__tests__/file.windows.test.ts that
covers the cases: "C:\\a\\b\\c.mp4" => ".mp4", "/a/b/c" => "", "/a/.env" => "",
"a.tar.gz" => ".gz", "a." => "", "" => "", and "C:/dir.with.dot/file" => "";
place tests under src/renderer/src/utils/__tests__/file.windows.test.ts, import
the renderer util, write assertions for each input->expected output, and run the
renderer test config to ensure they pass before opening the follow-up PR.
Edge case: dotfiles treated as having an extension; diverges from Node/path semantics and main getFileExt.
For filenames like ".bashrc", this returns ".bashrc" (non-empty), whereas Node's path.extname returns "" and your main getFileExt intends that behavior. Align to ignore a leading dot with no further dots and avoid future main/renderer divergence.
Apply:
- // 分割文件名以获取扩展名
- const parts = fileName.split('.')
- if (parts.length > 1) {
- const extension = parts.slice(-1)[0].toLowerCase()
-
- // 验证扩展名有效性：不能为空，不能包含路径分隔符
- if (extension && !extension.includes('/') && !extension.includes('\\')) {
- return '.' + extension
- }
- }
-
- return ''
+ // 使用最后一个 '.' 获取扩展名；忽略以 '.' 开头且无后续点的隐藏文件
+ const lastDot = fileName.lastIndexOf('.')
+ if (lastDot > 0 && lastDot < fileName.length - 1) {
+ const ext = fileName.slice(lastDot + 1).toLowerCase()
+ if (ext && !/[\/\\]/.test(ext)) {
+ return '.' + ext.replace(/^\.+/, '') // '..mp4' -> '.mp4'
+ }
+ }
+ return ''📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!filePath) { | |
| return '' | |
| } | |
| // 标准化路径分隔符和清理路径 | |
| let normalizedPath = filePath.trim() | |
| // 处理反斜杠（Windows 路径） | |
| normalizedPath = normalizedPath.replace(/\\/g, '/') | |
| // 获取文件名部分（移除路径） | |
| const fileName = normalizedPath.split('/').pop() || normalizedPath | |
| // 分割文件名以获取扩展名 | |
| const parts = fileName.split('.') | |
| if (parts.length > 1) { | |
| const extension = parts.slice(-1)[0].toLowerCase() | |
| return '.' + extension | |
| // 验证扩展名有效性：不能为空，不能包含路径分隔符 | |
| if (extension && !extension.includes('/') && !extension.includes('\\')) { | |
| return '.' + extension | |
| } | |
| } | |
| return '.' | |
| return '' | |
| } | |
| if (!filePath) { | |
| return '' | |
| } | |
| // 标准化路径分隔符和清理路径 | |
| let normalizedPath = filePath.trim() | |
| // 处理反斜杠（Windows 路径） | |
| normalizedPath = normalizedPath.replace(/\\/g, '/') | |
| // 获取文件名部分（移除路径） | |
| const fileName = normalizedPath.split('/').pop() || normalizedPath | |
| // 使用最后一个 '.' 获取扩展名；忽略以 '.' 开头且无后续点的隐藏文件 | |
| const lastDot = fileName.lastIndexOf('.') | |
| if (lastDot > 0 && lastDot < fileName.length - 1) { | |
| const ext = fileName.slice(lastDot + 1).toLowerCase() | |
| if (ext && !/[\/\\]/.test(ext)) { | |
| return '.' + ext.replace(/^\.+/, '') // '..mp4' -> '.mp4' | |
| } | |
| } | |
| return '' | |
| } |
🤖 Prompt for AI Agents
In src/renderer/src/utils/file.ts around lines 20 to 45, the current logic
treats dotfiles like ".bashrc" as having an extension; update the extension
detection to match Node/path.extname and the main getFileExt by ignoring a
single leading dot with no other dots. Concretely: after computing fileName and
parts, if fileName startsWith('.') and parts.length === 2 then return '' (no
extension); otherwise proceed with the existing validation and return '.' +
extension when valid.
2 participants
Summary
This PR contains comprehensive improvements including performance optimizations, bug fixes, new features, and development workflow enhancements.
New Features
Bug Fixes
Documentation
Development Workflow
Performance Improvements
Files Changed
Testing
Impact
This release significantly improves user experience through performance optimizations, resolves multiple reported issues, and establishes a robust development workflow for future releases.