(feat) Customize default project name, update project file names from UUID to pretty name #1372
Conversation
…owsCard + Fix button text spilling
WalkthroughAdds project-name templating, a Tauri formatting command, persisted default template UI, a one-time migration renaming UUID project folders to sanitized unique names, utilities for moment→chrono conversion and unique filenames (Aho‑Corasick), and related dependency and UI/type updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Frontend (Settings/Recorder UI)
participant Tauri as Tauri App
participant Recording as Recording Module
participant Utils as Utils crate
User->>Frontend: Edit default template / request preview
Frontend->>Tauri: formatProjectName(template, target, kind, mode, datetime)
Tauri->>Recording: format_project_name(...)
Recording->>Utils: moment_format_to_chrono(format)
Utils-->>Recording: chrono_format
Recording->>Recording: placeholder substitutions (Aho‑Corasick, regex)
Recording-->>Tauri: formatted project name
Tauri-->>Frontend: preview/display result
Note over Tauri,Recording: App startup one-time migration
Tauri->>Recording: migrate_if_needed()
Recording->>Recording: enumerate UUID-named project dirs
Recording->>Utils: ensure_unique_filename(pretty_name, parent_dir)
Utils-->>Recording: unique filename
Recording->>Recording: rename directory (or skip/fail)
Recording-->>Tauri: migration result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
crates/recording/src/sources/screen_capture/mod.rs (1)
197-203: LGTM! Clean implementation.The method correctly returns string representations for all
ScreenCaptureTargetvariants. The implementation is straightforward and appropriate for providing human-readable labels.Optional: Consider adding unit tests.
Since this is a new public method, you may want to add a simple test to verify the string representations:
#[cfg(test)] mod tests { use super::*; #[test] fn test_kind_str() { let display = ScreenCaptureTarget::Display { id: DisplayId::default() }; assert_eq!(display.kind_str(), "Display"); let window = ScreenCaptureTarget::Window { id: WindowId::default() }; assert_eq!(window.kind_str(), "Window"); let area = ScreenCaptureTarget::Area { screen: DisplayId::default(), bounds: LogicalBounds::new(LogicalPosition::new(0.0, 0.0), LogicalSize::new(100.0, 100.0)), }; assert_eq!(area.kind_str(), "Area"); } }apps/desktop/src/routes/(window-chrome)/settings/general.tsx (2)
653-660: Consider relaxing the minimum length constraint.The
isSaveDisabledfunction prevents saving templates withlength <= 3, which might be too restrictive. Valid templates like"{D}"(day of month) have only 3 characters but are legitimate use cases.Consider removing or reducing the minimum length check:
const isSaveDisabled = () => { const input = inputValue(); return ( !input || - input === (props.value ?? DEFAULT_PROJECT_NAME_TEMPLATE) || - input.length <= 3 + input === (props.value ?? DEFAULT_PROJECT_NAME_TEMPLATE) ); };Alternatively, validate that the template contains at least one placeholder instead of checking raw length.
735-741: Consider adding blur animation to Collapsible for smoother transitions.The PR description mentions adding blur to the collapsible animation. The current implementation uses opacity transitions, but you may want to add a blur effect for enhanced visual polish, consistent with the PR objectives.
Add blur to the animation if desired:
-<Collapsible.Content class="opacity-0 transition animate-collapsible-up ui-expanded:animate-collapsible-down ui-expanded:opacity-100 text-xs text-gray-12 space-y-3 px-1 pb-2"> +<Collapsible.Content class="opacity-0 blur-sm transition animate-collapsible-up ui-expanded:animate-collapsible-down ui-expanded:opacity-100 ui-expanded:blur-0 text-xs text-gray-12 space-y-3 px-1 pb-2">crates/utils/src/lib.rs (1)
200-222: Consider expanding test coverage for edge cases.The current test validates basic conversion and the borrowed case, but could benefit from additional test cases to ensure robustness.
Add tests for additional scenarios:
#[test] fn test_moment_format_edge_cases() { // Test overlapping patterns assert_eq!(moment_format_to_chrono("MMMMM"), "%BM"); // Test partial matches assert_eq!(moment_format_to_chrono("YYY"), "YY%y"); // Test all placeholder types assert_eq!(moment_format_to_chrono("hh:mm:ss A"), "%I:%M:%S %p"); assert_eq!(moment_format_to_chrono("HH:mm:ss"), "%H:%M:%S"); // Test no-op with special characters assert_eq!(moment_format_to_chrono("---///"), "---///"); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.toml(3 hunks)apps/desktop/src-tauri/Cargo.toml(4 hunks)apps/desktop/src-tauri/src/general_settings.rs(2 hunks)apps/desktop/src-tauri/src/lib.rs(4 hunks)apps/desktop/src-tauri/src/recording.rs(6 hunks)apps/desktop/src-tauri/src/update_project_names.rs(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(5 hunks)apps/desktop/src/utils/tauri.ts(2 hunks)crates/recording/src/sources/screen_capture/mod.rs(1 hunks)crates/utils/Cargo.toml(2 hunks)crates/utils/src/lib.rs(2 hunks)packages/ui-solid/src/auto-imports.d.ts(1 hunks)packages/ui-solid/tailwind.config.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
apps/desktop/src/utils/tauri.tspackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/routes/(window-chrome)/settings/general.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
apps/desktop/src/utils/tauri.tspackages/ui-solid/src/auto-imports.d.tspackages/ui-solid/tailwind.config.jsapps/desktop/src/routes/(window-chrome)/settings/general.tsx
**/tauri.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not edit auto-generated files named
tauri.ts.
Files:
apps/desktop/src/utils/tauri.ts
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/general_settings.rscrates/recording/src/sources/screen_capture/mod.rsapps/desktop/src-tauri/src/lib.rscrates/utils/src/lib.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/update_project_names.rs
crates/*/src/**/*
📄 CodeRabbit inference engine (AGENTS.md)
Rust crates should place tests within the
src/and/or a siblingtests/directory for each crate insidecrates/*.
Files:
crates/recording/src/sources/screen_capture/mod.rscrates/utils/src/lib.rs
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to **/tauri.ts : Do not edit auto-generated files named `tauri.ts`.
Applied to files:
apps/desktop/src/utils/tauri.ts
📚 Learning: 2025-09-22T14:19:56.010Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:56.010Z
Learning: Applies to packages/ui/**/*.{ts,tsx,js,jsx} : Component files in `packages/ui` should use PascalCase naming if they define React/Solid components.
Applied to files:
packages/ui-solid/src/auto-imports.d.ts
🧬 Code graph analysis (5)
crates/recording/src/sources/screen_capture/mod.rs (1)
apps/desktop/src/utils/tauri.ts (1)
ScreenCaptureTarget(470-470)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src-tauri/src/recording.rs (2)
format_project_name(305-378)target_name(183-188)apps/desktop/src-tauri/src/update_project_names.rs (1)
migrate_if_needed(12-37)
apps/desktop/src-tauri/src/recording.rs (2)
crates/utils/src/lib.rs (3)
ensure_dir(16-19)moment_format_to_chrono(166-198)ensure_unique_filename(45-91)apps/desktop/src-tauri/src/lib.rs (12)
std(1388-1406)std(1431-1453)format_project_name(2648-2662)new(349-351)new(766-770)new(1326-1356)app(1169-1170)app(2228-2228)app(2259-2259)app(2320-2320)app(2326-2326)app(2550-2551)
apps/desktop/src-tauri/src/update_project_names.rs (3)
apps/desktop/src-tauri/src/lib.rs (12)
std(1388-1406)std(1431-1453)recordings_path(2604-2608)app(1169-1170)app(2228-2228)app(2259-2259)app(2320-2320)app(2326-2326)app(2550-2551)new(349-351)new(766-770)new(1326-1356)crates/project/src/meta.rs (1)
load_for_project(147-153)crates/utils/src/lib.rs (1)
ensure_unique_filename(45-91)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (3)
apps/desktop/src/utils/tauri.ts (1)
commands(7-295)packages/ui-solid/src/Button.tsx (1)
Button(40-50)apps/desktop/src/routes/editor/ui.tsx (1)
Input(149-159)
🔇 Additional comments (16)
packages/ui-solid/src/auto-imports.d.ts (1)
1-11: This is an auto-generated file—verify it should be committed to version control.Line 5 indicates this file is generated by
unplugin-auto-import. Auto-generated files are typically excluded from version control (via.gitignore) and should not be manually edited. The quote style change on Line 11 likely stems from tool regeneration rather than an intentional code change.Before merging, confirm:
- Whether this file should be committed or added to
.gitignore- If committed, whether the changes come from an updated auto-import configuration or tool version
packages/ui-solid/tailwind.config.js (1)
92-97: LGTM! Blur effect added to collapsible animations.The blur transition enhances the visual feedback during collapse/expand animations. The implementation is consistent across both keyframes.
Note that blur filters can be GPU-intensive. If performance issues arise on lower-end devices with multiple collapsibles animating simultaneously, consider reducing the blur amount or making it optional.
crates/utils/Cargo.toml (1)
10-18: LGTM: Formatting and dependency additions are appropriate.The Windows feature list reformatting improves readability without changing functionality, and the aho-corasick workspace dependency aligns with the new string pattern matching utilities introduced in this crate.
apps/desktop/src-tauri/Cargo.toml (1)
22-28: LGTM: Dependency additions support new project naming features.The added dependencies (regex for pattern matching, sanitize-filename for safe filenames, and aho-corasick for efficient string replacement) are appropriate for the project name formatting functionality introduced in this PR.
Also applies to: 63-63, 110-110, 118-118
apps/desktop/src-tauri/src/general_settings.rs (1)
123-124: LGTM: New setting field is properly integrated.The
default_project_name_templatefield is correctly declared with#[serde(default)], ensuring backward compatibility with existing configurations. TheOption<String>type appropriately represents an optional user preference.Cargo.toml (1)
3-8: LGTM: Workspace configuration changes are well-structured.The multiline workspace members format improves maintainability. Adding "chrono" to specta features enables DateTime serialization for the format_project_name command, and the aho-corasick workspace dependency properly centralizes version management for the new string pattern matching functionality.
Also applies to: 30-30, 49-49
apps/desktop/src-tauri/src/lib.rs (4)
28-28: LGTM: Migration module properly integrated.The
update_project_namesmodule is appropriately added to support the one-time project name migration.
1904-1905: LGTM: New commands properly registered.The
format_project_namecommand is correctly registered in the Tauri command set alongside the existing commands.
2646-2662: LGTM: Command implementation correctly delegates to business logic.The
format_project_namecommand properly delegates to therecording::format_project_namefunction, converting the optional template to a string reference as needed.
2054-2054: No issues found — migration implementation is robust.The migration meets all three verification criteria:
Idempotent: Checked via
STORE_KEYon lines 19-25. If already migrated, the function returns immediately without re-running.Error logging: Migration failures are logged with
tracing::error!()on line 28, while store operation failures propagate with?operator on lines 17 and 33-34.Graceful degradation: Migration errors don't block app startup (lines 27-29 catch and log only). The
STORE_KEYis marked true regardless, preventing retry loops on future startups.The only errors that block startup are store access/save failures, which correctly indicate system-level issues that should prevent app initialization.
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (3)
104-105: LGTM: Default template is well-chosen.The default template
"{target_name} ({target_kind}) {date} {time}"provides a good balance of information and readability for project names.
507-512: LGTM: Component integration is clean.The
DefaultProjectNameCardis properly integrated into the settings with appropriate value binding and change handler.
883-883: LGTM: Layout fix prevents button text overflow.The
flex-shrink-0class properly fixes the layout issue where reset button text would spill out at certain viewport sizes, as mentioned in the PR objectives.crates/utils/src/lib.rs (2)
45-91: LGTM: Well-implemented filename uniqueness helper.The
ensure_unique_filenamefunction properly handles:
- Files without extensions
- Incremental numbering with clear formatting
- Early return when no conflict exists
- Reasonable loop limit (1000) to prevent infinite loops
The implementation is clear and well-documented.
166-198: LGTM: Efficient pattern conversion with good performance characteristics.The
moment_format_to_chronofunction demonstrates excellent design:
- Uses
LazyLockfor efficient static initialization of the automatonLeftmostLongestmatching correctly prioritizes longer patterns (e.g., "MMMM" before "MM")- Returns
Cow::Borrowedwhen no replacements are needed, avoiding unnecessary allocations- Comprehensive documentation with examples
apps/desktop/src/utils/tauri.ts (1)
292-294: DateTime serialization is properly configured.The chrono feature has been correctly added to specta in the workspace Cargo.toml (line 30), enabling proper serialization of
Option<chrono::DateTime<chrono::Local>>to TypeScript'sstring | null. The function implementation in tauri.ts passes the datetime parameter directly through TAURI_INVOKE, allowing specta with the chrono feature to handle serialization automatically. No additional verification or testing is required—the configuration is correct.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src-tauri/src/recording.rs (1)
263-324: The{target}placeholder concern appears addressed.The past review comment flagged that
{target}was mentioned in documentation but not implemented. The current documentation and example (line 290) correctly use{target_kind}and{target_name}as separate placeholders, which are implemented in the AhoCorasick patterns (lines 317-320). This appears to resolve the previous concern.
🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/recording.rs (2)
24-24: Consider usingstd::sync::LazyLockfor consistency.The codebase already uses
LazyLockinmoment_format_to_chrono(from the relevant code snippets). For consistency and to use the more modern standard library feature (stable since Rust 1.80), consider replacinglazy_static!withLazyLockin theformat_project_namefunction.
405-406: Consider removing redundant colon replacement.The
sanitize_filename::sanitize()call on line 406 should already handle filesystem-unsafe characters including colons. The explicit.replace(":", ".")on line 405 appears redundant, though it's harmless.If you prefer to keep the explicit replacement for clarity, this is fine. Otherwise, you can simplify to:
- let filename = project_name.replace(":", "."); - let filename = format!("{}.cap", sanitize_filename::sanitize(&filename)); + let filename = format!("{}.cap", sanitize_filename::sanitize(&project_name));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/recording.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/recording.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/recording.rs (2)
crates/utils/src/lib.rs (3)
ensure_dir(16-19)moment_format_to_chrono(166-198)ensure_unique_filename(45-91)apps/desktop/src-tauri/src/lib.rs (12)
std(1388-1406)std(1431-1453)format_project_name(2648-2662)new(349-351)new(766-770)new(1326-1356)app(1169-1170)app(2228-2228)app(2259-2259)app(2320-2320)app(2326-2326)app(2550-2551)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (1)
apps/desktop/src-tauri/src/recording.rs (1)
391-413: LGTM! Clean integration of template-based project naming.The integration of
format_project_namethroughoutstart_recordingis well-executed:
- Retrieves the template from settings
- Generates a sanitized, unique recording directory
- Propagates the project name to metadata and upload info
Note: The
target_namefield inInProgressRecordingCommon(line 590) now holds the computedproject_namerather than just the capture target's name, which extends its semantics while maintaining backward compatibility.Also applies to: 435-435, 476-476, 590-590
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src-tauri/src/recording.rs (1)
263-377: Consider usingLazyLockfor consistency and adding unit tests.The function correctly implements template formatting with multiple placeholder types. Two improvements to consider:
- Consistency with utils crate: The
cap_utilscrate usesstd::sync::LazyLock(seemoment_format_to_chrono), but this function useslazy_static!. For consistency, consider migrating toLazyLock:-use lazy_static::lazy_static; +use std::sync::LazyLock; - lazy_static! { - static ref DATE_REGEX: Regex = Regex::new(r"\{date(?::([^}]+))?\}").unwrap(); - static ref TIME_REGEX: Regex = Regex::new(r"\{time(?::([^}]+))?\}").unwrap(); - static ref MOMENT_REGEX: Regex = Regex::new(r"\{moment(?::([^}]+))?\}").unwrap(); - static ref AC: aho_corasick::AhoCorasick = { + static DATE_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{date(?::([^}]+))?\}").unwrap()); + static TIME_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{time(?::([^}]+))?\}").unwrap()); + static MOMENT_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{moment(?::([^}]+))?\}").unwrap()); + static AC: LazyLock<aho_corasick::AhoCorasick> = LazyLock::new(|| { aho_corasick::AhoCorasick::new(&[ "{recording_mode}", "{mode}", "{target_kind}", "{target_name}", ]) .expect("Failed to build AhoCorasick automaton") - }; - } + });
- Unit test coverage: This function has multiple code paths (different placeholders, custom format specifiers, edge cases). Consider adding unit tests in the
testsmodule to verify behavior with various templates, format specifiers, and edge cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/recording.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/recording.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/recording.rs (1)
crates/utils/src/lib.rs (3)
ensure_dir(16-19)moment_format_to_chrono(166-198)ensure_unique_filename(45-91)
🔇 Additional comments (2)
apps/desktop/src-tauri/src/recording.rs (2)
22-28: LGTM! Imports are appropriate for the new template formatting functionality.The new imports support the template parsing and formatting infrastructure.
394-416: LGTM! Project name formatting and sanitization flow is well-structured.The integration correctly formats the project name from the template, sanitizes it for filesystem compatibility, and ensures uniqueness. The explicit colon-to-period replacement before sanitization adds clarity even though
sanitize_filenamemight handle it.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/desktop/src-tauri/Cargo.toml (1)
114-114: Clarify the commented feature syntax on line 114.Line 114 has trailing commented-out code that appears to be incomplete feature specifications. This may confuse future maintainers. Consider either removing this comment or clarifying its intent.
-opentelemetry-otlp = "0.31.0" #{ version = , features = ["http-proto", "reqwest-client"] } +opentelemetry-otlp = "0.31.0"apps/desktop/src-tauri/src/recording.rs (3)
24-24: Consider usingstd::sync::LazyLockfor consistency.The
lazy_staticcrate is used here, but thecap_utilscrate usesstd::sync::LazyLock(stable since Rust 1.80). UsingLazyLockwould improve consistency across the codebase and reduce external dependencies.Apply this diff to use
LazyLock:-use lazy_static::lazy_static; +use std::sync::LazyLock;Then update the lazy initialization in
format_project_name(around line 314):- lazy_static! { - static ref DATE_REGEX: Regex = Regex::new(r"\{date(?::([^}]+))?\}").unwrap(); - static ref TIME_REGEX: Regex = Regex::new(r"\{time(?::([^}]+))?\}").unwrap(); - static ref MOMENT_REGEX: Regex = Regex::new(r"\{moment(?::([^}]+))?\}").unwrap(); - static ref AC: aho_corasick::AhoCorasick = { + static DATE_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{date(?::([^}]+))?\}").unwrap()); + static TIME_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{time(?::([^}]+))?\}").unwrap()); + static MOMENT_REGEX: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{moment(?::([^}]+))?\}").unwrap()); + static AC: LazyLock<aho_corasick::AhoCorasick> = LazyLock::new(|| { aho_corasick::AhoCorasick::new(&[ "{recording_mode}", "{mode}", "{target_kind}", "{target_name}", ]) .expect("Failed to build AhoCorasick automaton") - }; - } + });Based on learnings
263-377: Consider adding unit tests forformat_project_name.This is a core public function exposed as a Tauri command with complex formatting logic involving multiple token replacements. Unit tests would help ensure correctness and prevent regressions, especially for edge cases like:
- Empty or missing template
- Custom date/time formats with moment syntax
- All placeholder combinations
- Special characters in target names
Example test structure:
#[cfg(test)] mod format_project_name_tests { use super::*; use chrono::TimeZone; #[test] fn test_default_template() { let dt = chrono::Local.with_ymd_and_hms(2025, 11, 12, 15, 23, 0).unwrap(); let result = format_project_name( None, "Built-in Retina Display", "Display", RecordingMode::Instant, Some(dt), ); assert!(result.contains("Built-in Retina Display")); assert!(result.contains("Display")); assert!(result.contains("2025-11-12")); } #[test] fn test_custom_moment_format() { let dt = chrono::Local.with_ymd_and_hms(2025, 11, 12, 15, 23, 0).unwrap(); let result = format_project_name( Some("{moment:YYYY-MM-DD HH:mm}"), "Test", "Window", RecordingMode::Studio, Some(dt), ); assert!(result.contains("2025-11-12 15:23")); } #[test] fn test_all_placeholders() { let result = format_project_name( Some("{recording_mode} {mode} {target_kind} {target_name}"), "Chrome", "Window", RecordingMode::Studio, None, ); assert_eq!(result, "Studio studio Window Chrome"); } }
593-593: Consider renamingtarget_namefield toproject_namefor clarity.The
target_namefield inInProgressRecordingCommonis now populated with the formattedproject_namerather than the raw capture target name. This semantic mismatch could confuse future maintainers. While this works functionally, renaming the field toproject_namethroughout the structs (InProgressRecordingCommon,CompletedRecording) would better reflect its actual purpose.Note: This would be a broader refactor touching multiple files and is not urgent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/desktop/src-tauri/Cargo.toml(4 hunks)apps/desktop/src-tauri/src/recording.rs(6 hunks)apps/desktop/src/utils/tauri.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/utils/tauri.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/recording.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/recording.rs (1)
crates/utils/src/lib.rs (3)
ensure_dir(16-19)moment_format_to_chrono(166-198)ensure_unique_filename(45-91)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (2)
apps/desktop/src-tauri/Cargo.toml (1)
63-63: All three new dependencies are properly configured and verified.The workspace dependency
aho-corasick = "1.1.4"is correctly defined in the root Cargo.toml (line 49), and the reference at line 118 (aho-corasick.workspace = true) is valid. All three new dependencies are stable, current, and free of known security issues:
regex = "1.10.4"— Well above the CVE-2022-24713 fix threshold (1.5.5)sanitize-filename = "0.6.0"— Latest stable versionaho-corasick— Properly defined in workspace and compatibleThe feature formatting changes are non-functional.
apps/desktop/src-tauri/src/recording.rs (1)
394-416: LGTM! Clean integration of project name formatting.The workflow correctly:
- Formats the project name using the template
- Sanitizes colons and filesystem-unsafe characters
- Ensures uniqueness in the recordings directory
- Creates the directory
The colon replacement (line 408) before sanitization is a good defensive measure for cross-platform compatibility.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (1)
105-107: Keep TS and Rust default templates in sync
DEFAULT_PROJECT_NAME_TEMPLATEmirrors the RustDEFAULT_FILENAME_TEMPLATEinrecording::format_project_name. To avoid future drift, consider adding a brief comment referencing the Rust constant (or a test that asserts both defaults format the same example) so changes on one side don’t silently desync.apps/desktop/src-tauri/src/recording.rs (1)
720-724: Field nametarget_namenow holds project name
InProgressRecordingCommon { target_name: project_name, .. }repurposes thetarget_namefield to actually carry the formatted project name. That’s fine behaviorally (the field is only used to plumb a display name throughCompletedRecording), but the naming is now misleading.If you touch this area again, consider renaming the field (and
CompletedRecording::target_name) to something likeproject_nameto better reflect its semantics; no urgent change needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
apps/desktop/src-tauri/Cargo.toml(4 hunks)apps/desktop/src-tauri/src/general_settings.rs(2 hunks)apps/desktop/src-tauri/src/lib.rs(4 hunks)apps/desktop/src-tauri/src/recording.rs(7 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(5 hunks)apps/desktop/src/utils/tauri.ts(2 hunks)packages/ui-solid/src/auto-imports.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src-tauri/src/general_settings.rs
- apps/desktop/src-tauri/Cargo.toml
- apps/desktop/src/utils/tauri.ts
🧰 Additional context used
🧬 Code graph analysis (3)
apps/desktop/src-tauri/src/recording.rs (2)
crates/utils/src/lib.rs (3)
ensure_dir(16-19)moment_format_to_chrono(166-198)ensure_unique_filename(45-91)apps/desktop/src-tauri/src/lib.rs (12)
std(1785-1803)std(1828-1850)format_project_name(3060-3074)new(730-732)new(1163-1167)new(1723-1753)app(1566-1567)app(2629-2629)app(2660-2660)app(2725-2725)app(2731-2731)app(2962-2963)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src-tauri/src/recording.rs (2)
format_project_name(396-469)target_name(266-271)apps/desktop/src-tauri/src/update_project_names.rs (1)
migrate_if_needed(12-37)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (3)
apps/desktop/src/utils/tauri.ts (1)
commands(7-298)packages/ui-solid/src/Button.tsx (1)
Button(40-50)apps/desktop/src/routes/editor/ui.tsx (1)
Input(149-159)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (6)
packages/ui-solid/src/auto-imports.d.ts (1)
11-11: LGTM! Auto-generated icon import follows the correct pattern.The new
IconCapAutodeclaration is correctly formatted and alphabetically ordered among the other icon imports. Since this file is auto-generated byunplugin-auto-import, the addition appears consistent with the tooling's expected output.apps/desktop/src/routes/(window-chrome)/settings/general.tsx (3)
9-22: New imports and hooks look consistent with usage
Collapsible,createSignal, andonMountare all used inDefaultProjectNameCard; nothing appears unused or mis-imported. No changes needed here.
774-835: Collapsible “How to customize?” section aligns with backend placeholdersThe placeholder descriptions ({recording_mode}, {mode}, {target_kind}, {target_name}, {date}, {time},
{moment:…}) match the Rust implementation offormat_project_nameand themoment_format_to_chronobehavior. This should give users an accurate mental model of what’s supported.
922-944: Excluded Windows header layout fix is correctAdding
flex-shrink-0to the action container prevents the reset/add buttons from compressing or wrapping in narrow viewports, which addresses the earlier text spill without side effects elsewhere in the card.apps/desktop/src-tauri/src/lib.rs (1)
2302-2304:format_project_nameTauri command wiring looks correctThe new
format_project_namecommand is registered in the specta builder and cleanly delegates torecording::format_project_name, convertingOption<String>/Stringarguments into theOption<&str>/&strexpected by the core function. This should interoperate correctly with the generated TS bindings and the frontend calls inutils/tauri.ts.Also applies to: 3058-3074
apps/desktop/src-tauri/src/recording.rs (1)
486-511: New project-name templating and filename derivation look correctThe new
start_recordingflow:
- Derives
project_nameviaformat_project_name, usingdefault_project_name_templatewhen present and falling back to the hard-coded default.- Builds a filesystem-safe, unique directory name by:
- Replacing
:with.,- Passing through
sanitize_filename,- Appending
.cap,- Running
ensure_unique_filenameunderapp_data_dir()/recordings.This cleanly separates human-readable naming from the underlying folder layout and avoids collisions. The subsequent
ensure_dir(&recording_dir)+RecordingMeta { pretty_name: project_name.clone(), .. }wiring is consistent with downstream uses (uploads, editor, listing).
| use cap_utils::{ensure_dir, moment_format_to_chrono, spawn_actor}; | ||
| use futures::{FutureExt, stream}; | ||
| use lazy_static::lazy_static; | ||
| use regex::Regex; | ||
| use serde::{Deserialize, Serialize}; | ||
| use specta::Type; | ||
| use std::borrow::Cow; | ||
| use std::{ |
There was a problem hiding this comment.
format_project_name implementation is solid; fix minor time-format doc mismatch
The new format_project_name helper is well-structured:
- Uses a pre-built Aho-Corasick automaton for
{recording_mode},{mode},{target_kind},{target_name}replacements. - Supports
{date},{time}, and{moment}with optional custom moment-style formats, mapping them viamoment_format_to_chronobefore passing tochrono::DateTime::format. - Keeps a sensible default template (
{target_name} ({target_kind}) {date} {time}) that matches the TSDEFAULT_PROJECT_NAME_TEMPLATE.
One minor nit: the default time format in code is %I:%M %p (Line 451), which produces zero-padded hours (e.g., "03:23 PM"), but the doc comment and UI examples show "3:23 PM". To avoid confusion either:
- Change the default to
%-I:%M %pto match the example, or - Adjust the example/docs to show a zero-padded hour.
Functionally everything else looks correct.
Also applies to: 355-469
🤖 Prompt for AI Agents
In apps/desktop/src-tauri/src/recording.rs around lines 355-469 (imports are at
top around 26-33), the default time format uses "%I:%M %p" which zero-pads hours
(e.g., "03:23 PM") but the doc comment and UI examples show a non-padded hour
("3:23 PM"); change the default time format string to "%-I:%M %p" to produce
non-padded hours and update the nearby doc comment/example if present so they
match the new default.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/desktop/src-tauri/src/update_project_names.rs (2)
27-34: Critical: Migration flag still set on failure (past issue remains)The past review comment correctly identified that lines 27-34 mark the migration as complete even when
migratereturns an error. This means:
- If migration fails for any reason (permissions, conflicts, etc.), the flag is still set
- Future app launches will never retry the migration
- Users will permanently have UUID-based folders
100-138: Major: Partial failures still swallowed (past issue remains)The past review correctly identified that this function returns
Ok(())even whenfailed > 0. Combined with the flag-setting issue inmigrate_if_needed, a single failure permanently blocks future migration attempts.The detailed performance logging (wall time, averages, top slowest projects) is excellent for observability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/update_project_names.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/update_project_names.rs (3)
apps/desktop/src-tauri/src/lib.rs (12)
std(1785-1803)std(1828-1850)recordings_path(3016-3020)app(1566-1567)app(2629-2629)app(2660-2660)app(2725-2725)app(2731-2731)app(2962-2963)new(730-732)new(1163-1167)new(1723-1753)crates/project/src/meta.rs (1)
load_for_project(147-153)crates/utils/src/lib.rs (1)
ensure_unique_filename(45-91)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (5)
apps/desktop/src-tauri/src/update_project_names.rs (5)
141-167: LGTM: UUID project collection logic is soundThe async directory enumeration correctly filters for UUID-formatted
.capdirectories using the validation helper. Error propagation is appropriate.
175-204: LGTM: Per-project migration logic handles errors appropriatelyThe function properly loads metadata, migrates the filename, and returns appropriate results (Migrated/Skipped) based on whether the path changed. Error logging includes context.
211-211: Verify the colon pre-replacement before sanitizationThe code replaces colons with periods before calling
sanitize_filename::sanitize. This enforces a specific formatting choice (e.g., timestamps like "2024-01-01 12:00:00" become "2024-01-01 12.00.00").Since
sanitize_filenamewould handle colons automatically (possibly differently per platform), verify whether this pre-replacement is:
- Intentional to maintain consistent readability across platforms
- Redundant and could be removed
- Incorrect and should use sanitize's default handling
Consider requesting clarification from the PR author or checking if there's a design document explaining the naming format.
235-251: LGTM: UUID validation is correct and efficientThe function correctly validates:
- Overall length (36 for UUID + 4 for ".cap")
- Hyphen positions at indices 8, 13, 18, 23 (standard UUID format: 8-4-4-4-12)
- All characters are ASCII hex digits or hyphens
The name
fast_is_project_filename_uuidaccurately reflects the optimized approach (byte-level checks before character iteration).
253-279: LGTM: Test coverage validates key scenariosThe tests cover:
- Valid UUID filenames (varied hex, all-zeros edge case)
- Invalid formats (non-UUID string, wrong/missing extension, invalid hex character)
Coverage is adequate for the validation logic. Optional enhancements could include wrong hyphen positions or length variations, but current tests are sufficient.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/utils/src/lib.rs (2)
45-100: Clarify failure behavior and validateattemptsinensure_unique_filename*_with_attempts.The core logic looks solid, but a couple of edge cases are underspecified:
ensure_unique_filename_with_attemptsaccepts anyi32forattempts. Forattempts <= 0, the loop will error immediately (after at most one probe) with"Too many filename conflicts", even when there are no real conflicts.- The doc comment on
ensure_unique_filenamepromises “Returns the unique filename that doesn't conflict with existing files” but doesn’t mention that the function can fail when the suffix limit is exceeded.Consider tightening this by:
- Validating
attemptsat the top (e.g.,debug_assert!(attempts > 0)or returning an explicit error ifattempts <= 0), and- Updating the docs to describe the error case (and that
attemptsbounds the highest numeric suffix tried).Optionally, if
base_filenameis always expected to be a bare file name (no separators), adebug_assert!(!base_filename.contains(std::path::MAIN_SEPARATOR))would encode that assumption.
209-231: Extend tests to cover documented edge cases (e.g., ISO week and invalid/unknown tokens).The current test nicely exercises a mixed format and the borrowed/unchanged path. Given the richer doc examples, you might want to add:
- A test for the ISO week example (once the implementation or docs are corrected) so the mismatch can’t regress silently.
- One or two tests around “near misses” (e.g., strings containing partial tokens or literals adjacent to tokens) to lock in the LeftmostLongest behavior.
This would make the behavior of
moment_format_to_chronomuch clearer and future refactors safer.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src-tauri/src/update_project_names.rs(1 hunks)crates/utils/src/lib.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/src/update_project_names.rs
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
crates/utils/src/lib.rs (1)
114-182: Address moment.js compatibility confusion flagged in previous review.The previous review flagged that this function claims to convert "moment template format" but uses incompatible token semantics. This concern was not fully addressed:
- Moment.js convention:
DDD/DDDD= day-of-year (1-365, 001-365);ddd/dddd= weekday names- This implementation:
DDD/DDDD= weekday names; day-of-year tokens are unsupportedThe current documentation still refers to "moment template format" (line 114) without clarifying this is a custom subset with different semantics. Users expecting moment.js compatibility will encounter unexpected behavior.
Based on previous review comments, either:
- Update the function description and docs to explicitly state this is a "moment-style subset" with a clear notice about the divergence:
-/// Converts user-friendly moment template format strings to chrono format strings. +/// Converts moment-style template format strings to chrono format strings. /// -/// This function translates common template format patterns to chrono format specifiers, -/// allowing users to write intuitive date/time formats that get converted to chrono's format. +/// This function translates a custom subset of date/time patterns to chrono format specifiers. +/// +/// **Note**: This is NOT fully compatible with moment.js. Notably, `DDD`/`DDDD` map to +/// weekday names here, whereas in moment.js they represent day-of-year. Day-of-year and +/// ISO week tokens are not supported.
- Or realign the tokens to match moment.js if compatibility is important (add
ddd/ddddfor weekdays, repurposeDDD/DDDDfor day-of-year).apps/desktop/src/routes/(window-chrome)/settings/general.tsx (1)
9-9: Default project name card wiring looks solid; consider debouncing preview updatesThe new
DefaultProjectNameCardis well-integrated: theDEFAULT_PROJECT_NAME_TEMPLATEfallback,value/onChangewiring intodefaultProjectNameTemplate, Save/Reset behavior, and the help section with copyable placeholders all read correctly, and the Save handler now correctly awaits persistence and preview updates.One remaining improvement is avoiding a Tauri round-trip on every keystroke in
onInputviaupdatePreview(e.currentTarget.value). A small trailing-edge debounce (e.g., 200–300ms) would reduce redundantformatProjectNamecalls for fast typers and also mitigate the chance of a slower earlier call overwriting a newer preview.For example:
function DefaultProjectNameCard(props: { @@ - const [momentExample, setMomentExample] = createSignal(""); + const [momentExample, setMomentExample] = createSignal(""); + let previewTimeoutId: number | undefined; @@ - async function updatePreview(val = inputValue()) { + async function updatePreview(val = inputValue()) { const formatted = await commands.formatProjectName( val, @@ setPreview(formatted); } + + function schedulePreviewUpdate(val: string) { + if (previewTimeoutId !== undefined) { + clearTimeout(previewTimeoutId); + } + previewTimeoutId = window.setTimeout(() => { + // Best-effort; log if something goes wrong instead of throwing in the UI event loop. + void updatePreview(val).catch((error) => { + console.error("Failed to update project name preview", error); + }); + }, 250); + } @@ - onInput={(e) => { - setInputValue(e.currentTarget.value); - updatePreview(e.currentTarget.value); - }} + onInput={(e) => { + const next = e.currentTarget.value; + setInputValue(next); + schedulePreviewUpdate(next); + }}This keeps the explicit
await updatePreview(...)calls for Reset/Save semantics while making the live preview cheaper and more robust during typing.Also applies to: 105-107, 546-551, 638-838
🧹 Nitpick comments (1)
crates/utils/src/lib.rs (1)
217-239: Consider expanding test coverage.The current tests only cover
moment_format_to_chronowith basic scenarios. Consider adding:
- Tests for
ensure_unique_filenameandensure_unique_filename_with_attemptsto verify:
- Behavior when file doesn't exist (returns base filename)
- Conflict resolution (appending numbers)
- Extension handling (files with/without extensions)
- Max attempts limit enforcement
- Additional
moment_format_to_chronotests for:
- Overlapping pattern handling (e.g., "MMMM" vs "MMM" precedence)
- Mixed token and literal text
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src-tauri/src/update_project_names.rs(1 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(5 hunks)crates/utils/src/lib.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src-tauri/src/update_project_names.rs
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (3)
apps/desktop/src/utils/tauri.ts (1)
commands(7-298)packages/ui-solid/src/Button.tsx (1)
Button(40-50)apps/desktop/src/routes/editor/ui.tsx (1)
Input(149-159)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (2)
crates/utils/src/lib.rs (1)
153-153: No action needed — chrono's%Pspecifier is cross-platform compatible.The web search confirms that Chrono implements the
%Pformat specifier natively (not delegated to platform libc), ensuring consistent lowercase "am"/"pm" output across Windows, macOS, and Linux. The documentation mapping of lowercaseato%Pis correct and safe.apps/desktop/src/routes/(window-chrome)/settings/general.tsx (1)
922-944: Excluded windows action row layout tweak is safe and improves resilienceChanging the action container to
class="flex flex-shrink-0 gap-2"is a low-risk layout fix: it keeps the Reset/Add buttons from shrinking and prevents the "Reset to Default" label from wrapping or clipping at narrower widths, without altering any behavior or event handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src-tauri/src/lib.rs (1)
3062-3078: LGTM! Command implementation is correct.The command properly delegates to
recording::format_project_namewith correct parameter type conversions. The async marker is unnecessary since no async operations are performed, but this is a minor inefficiency and may be intentional for consistency with other commands.If desired, you can remove the
asyncmarker for a slight performance improvement:-#[tauri::command] -#[specta::specta] -async fn format_project_name( +#[tauri::command(async)] +#[specta::specta] +fn format_project_name( template: Option<String>, target_name: String, target_kind: String, recording_mode: RecordingMode, datetime: Option<chrono::DateTime<chrono::Local>>, ) -> String {Note: The
#[tauri::command(async)]attribute makes the command callable asynchronously from the frontend while keeping the Rust implementation synchronous.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/lib.rs(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src-tauri/src/recording.rs (2)
format_project_name(396-469)target_name(266-271)apps/desktop/src-tauri/src/update_project_names.rs (1)
migrate_if_needed(16-41)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (3)
apps/desktop/src-tauri/src/lib.rs (3)
28-28: LGTM! Module declaration is appropriate.The new
update_project_namesmodule is correctly declared and used in the setup function to perform one-time project folder migration.
2447-2450: Past review concern has been addressed.The migration error handling has been updated since the previous review. The code now correctly catches and logs migration errors without aborting app startup, which is appropriate for this cosmetic one-time rename operation.
2303-2303: LGTM! Command registration is correct.The
format_project_namecommand is properly registered in the specta builder and will be available to the frontend via TypeScript bindings.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/desktop/src-tauri/src/recording.rs (1)
451-451: Consider non-padded hour format for consistency.The default time format uses
%I:%M %p, which produces zero-padded hours (e.g., "03:23 PM"). If the intent is to display non-padded hours (e.g., "3:23 PM"), use%-I:%M %pinstead.Based on past review comment that noted this discrepancy.
Apply this diff if non-padded hours are preferred:
- .unwrap_or(Cow::Borrowed("%I:%M %p")), + .unwrap_or(Cow::Borrowed("%-I:%M %p")),
🧹 Nitpick comments (1)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (1)
794-796: Consider debouncing the preview updatesThe
onInputhandler triggersupdatePreviewon every keystroke, which invokes a Tauri command. As noted in the previous review, adding a trailing-edge debounce (200-300ms) would reduce redundant round-trips during fast typing without changing the perceived behavior.Apply this approach to add debouncing:
+const [debounceTimer, setDebounceTimer] = createSignal<number | undefined>(); + async function updatePreview(val = inputValue()) { const formatted = await commands.formatProjectName( val, macos ? "Safari" : "Chrome", "Window", "instant", datetime, ); setPreview(formatted); }Then update the onInput handler:
onInput={(e) => { setInputValue(e.currentTarget.value); - updatePreview(e.currentTarget.value); + const timer = debounceTimer(); + if (timer !== undefined) clearTimeout(timer); + setDebounceTimer(setTimeout(() => { + updatePreview(e.currentTarget.value); + }, 250) as unknown as number); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/desktop/src-tauri/Cargo.toml(4 hunks)apps/desktop/src-tauri/src/lib.rs(4 hunks)apps/desktop/src-tauri/src/recording.rs(7 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(5 hunks)apps/desktop/src/utils/tauri.ts(2 hunks)packages/ui-solid/src/auto-imports.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/desktop/src-tauri/Cargo.toml
- apps/desktop/src/utils/tauri.ts
- packages/ui-solid/src/auto-imports.d.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx,js,jsx,rs,py,sh}
📄 CodeRabbit inference engine (CLAUDE.md)
Never add any form of comments to code (single-line //, multi-line /* /, documentation ///, //!, /* */, or any other comment syntax). Code must be self-explanatory through naming, types, and structure.
Files:
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src/routes/(window-chrome)/settings/general.tsx
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use Rust 2024 edition and run cargo fmt to format all Rust code with rustfmt
**/*.rs: Format all Rust code usingrustfmtand apply workspace clippy lints
Use snake_case for Rust module names and kebab-case for Rust crate names
Files:
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/recording.rs
apps/desktop/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Follow architectural decision to use tauri_specta for strongly typed Rust-to-TypeScript IPC bindings; do not modify generated bindings
Files:
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/recording.rs
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,rs}: Use kebab-case for file names (e.g.,user-menu.tsx)
Never add comments to code in any language (no//,/* */,///,//!,#, etc.). Code must be self-explanatory through naming, types, and structure
Files:
apps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src/routes/(window-chrome)/settings/general.tsx
apps/desktop/src/**/*.{ts,tsx,solid}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/src/**/*.{ts,tsx,solid}: Use @tanstack/solid-query for server state in desktop app; call generated commands and events from tauri_specta
Never manually import desktop app icons; use unplugin-icons auto-import instead
Files:
apps/desktop/src/routes/(window-chrome)/settings/general.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use PascalCase for component names; use camelCase starting with 'use' for hook names
Use PascalCase for component names in TypeScript/JavaScript
Files:
apps/desktop/src/routes/(window-chrome)/settings/general.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce strict TypeScript; avoid 'any' type; leverage shared types from packages/utils and other shared packages
Files:
apps/desktop/src/routes/(window-chrome)/settings/general.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Biome for linting and formatting; run pnpm format and pnpm lint regularly and at the end of each coding session
**/*.{ts,tsx,js,jsx}: Use 2-space indentation in TypeScript files
Format all TypeScript and JavaScript code using Biome (viapnpm format)
Files:
apps/desktop/src/routes/(window-chrome)/settings/general.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-29T04:31:05.289Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T04:31:05.289Z
Learning: Applies to apps/desktop/**/*.rs : Follow architectural decision to use tauri_specta for strongly typed Rust-to-TypeScript IPC bindings; do not modify generated bindings
Applied to files:
apps/desktop/src-tauri/src/lib.rs
📚 Learning: 2025-11-29T04:31:05.289Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T04:31:05.289Z
Learning: Applies to apps/desktop/**/*.ts : Never edit auto-generated files: tauri.ts, queries.ts, and files under apps/desktop/src-tauri/gen/
Applied to files:
apps/desktop/src/routes/(window-chrome)/settings/general.tsx
🧬 Code graph analysis (2)
apps/desktop/src-tauri/src/lib.rs (4)
apps/desktop/src-tauri/src/recording.rs (3)
format_project_name(396-469)app(1179-1179)target_name(266-271)apps/desktop/src-tauri/src/update_project_names.rs (1)
migrate_if_needed(16-41)apps/desktop/src-tauri/src/upload.rs (2)
app(712-713)app(794-795)apps/desktop/src/utils/tauri.ts (1)
RecordingMode(475-475)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (3)
apps/desktop/src/utils/tauri.ts (1)
commands(7-310)packages/ui-solid/src/Button.tsx (1)
Button(40-50)apps/desktop/src/routes/editor/ui.tsx (1)
Input(151-161)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (11)
apps/desktop/src-tauri/src/recording.rs (2)
26-33: LGTM!The new imports support the project name formatting functionality and are appropriately scoped.
486-508: LGTM!The integration of
format_project_nameinto the recording flow is well-structured:
- Properly retrieves template from settings
- Generates project name from template
- Sanitizes and ensures filename uniqueness
- Consistently uses the generated name throughout the recording setup
Also applies to: 530-530, 572-572, 725-725
apps/desktop/src-tauri/src/lib.rs (4)
29-29: LGTM!Module declaration is correct and necessary for the project name migration functionality.
2355-2356: LGTM!The
format_project_namecommand is correctly registered in the tauri_specta command list, following the established pattern.
2502-2504: LGTM!The migration error handling is correct. By logging the error rather than propagating it, the app can start even if the one-time project name migration fails. This prevents the migration from blocking users.
3126-3142: LGTM!The tauri command implementation is well-structured:
- Properly annotated for async tauri command and specta type generation
- Correctly converts owned string parameters to references when delegating to
recording::format_project_name- No coding guideline violations (no comments)
- Appropriate signature for IPC boundary
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (5)
9-9: LGTM: Imports and constant declarationThe new imports and default template constant are appropriate for the DefaultProjectNameCard component functionality.
Also applies to: 17-17, 19-19, 108-109
577-582: LGTM: Component integrationThe DefaultProjectNameCard is properly integrated with the settings flow, correctly wiring the persisted value and change handler.
777-780: Good: Save handler properly awaits async operationsThe Save button handler correctly awaits both
props.onChangeandupdatePreview, addressing the concern from the previous review. This ensures the UI preview reflects the persisted state.
953-953: Good fix: Prevent button container from shrinkingAdding
flex-shrink-0prevents the button text from overflowing at certain viewport sizes, as noted in the PR objectives.
669-869: Well-structured component with comprehensive documentationThe DefaultProjectNameCard component is well-implemented with:
- Clear state management using signals
- Live preview functionality via the formatProjectName command
- Helpful collapsible documentation with interactive code examples
- Proper async handling in button callbacks
The component provides a good user experience for customizing project name templates.
| const isSaveDisabled = () => { | ||
| const input = inputValue(); | ||
| return ( | ||
| !input || | ||
| input === (props.value ?? DEFAULT_PROJECT_NAME_TEMPLATE) || | ||
| input.length <= 3 | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Arbitrary length threshold in save validation
The isSaveDisabled check uses input.length <= 3 as one of its conditions. This threshold appears arbitrary—it would block "Hi" but allow "Test", neither of which is necessarily a valid template format. Consider either removing this check entirely (relying on the empty check and template validation in the backend) or adding a more meaningful validation (e.g., checking for at least one placeholder token).
🤖 Prompt for AI Agents
In apps/desktop/src/routes/(window-chrome)/settings/general.tsx around lines 723
to 730, the save-disable logic uses an arbitrary length check (`input.length <=
3`) which should be replaced with a meaningful validation: remove the length
threshold and instead ensure the template is non-empty, not equal to the
current/default value, and contains at least one placeholder token (e.g. match a
pattern like `{token}`) before allowing save; keep backend validation but update
this frontend check to look for a placeholder token (regex `\{[^}]+\}`) rather
than relying on string length.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src-tauri/src/recording.rs (2)
355-429: Consider validating template output for empty/whitespace-only results.The function doesn't validate whether the processed template produces a non-empty string. If a user provides an unusual template (e.g., only whitespace or placeholders that resolve to empty strings), the downstream sanitization on Line 461 might produce an empty or minimal filename, potentially causing issues with directory creation.
Consider adding validation after Line 428:
result.into_owned() + .trim() + .to_string() + .or_else(|s| if s.is_empty() { + "{target_name} ({target_kind}) {date} {time}".to_string() + } else { + s + }) }Actually, a simpler approach:
- result.into_owned() + let result = result.into_owned(); + if result.trim().is_empty() { + format_project_name( + Some(DEFAULT_FILENAME_TEMPLATE), + target_name, + target_kind, + recording_mode, + Some(datetime), + ) + } else { + result + } }
683-687: Project name correctly assigned to recording common structure.Setting
common.target_nametoproject_name(Line 684) propagates the formatted name throughout the recording flow. While the field nametarget_namemight suggest it holds the capture target's name, it effectively serves as the recording's identifier, so this usage is functionally correct.For future clarity, consider renaming
InProgressRecordingCommon.target_nametoInProgressRecordingCommon.recording_nameorproject_nameto better reflect its purpose. This is a low-priority refactor since the current usage is correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src-tauri/src/recording.rs(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,rs,py,sh}
📄 CodeRabbit inference engine (CLAUDE.md)
Never add any form of comments to code (single-line //, multi-line /* /, documentation ///, //!, /* */, or any other comment syntax). Code must be self-explanatory through naming, types, and structure.
Files:
apps/desktop/src-tauri/src/recording.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use Rust 2024 edition and run cargo fmt to format all Rust code with rustfmt
**/*.rs: Format all Rust code usingrustfmtand apply workspace clippy lints
Use snake_case for Rust module names and kebab-case for Rust crate names
Files:
apps/desktop/src-tauri/src/recording.rs
apps/desktop/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Follow architectural decision to use tauri_specta for strongly typed Rust-to-TypeScript IPC bindings; do not modify generated bindings
Files:
apps/desktop/src-tauri/src/recording.rs
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,rs}: Use kebab-case for file names (e.g.,user-menu.tsx)
Never add comments to code in any language (no//,/* */,///,//!,#, etc.). Code must be self-explanatory through naming, types, and structure
Files:
apps/desktop/src-tauri/src/recording.rs
🧠 Learnings (2)
📚 Learning: 2025-11-29T04:31:05.289Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T04:31:05.289Z
Learning: Applies to **/*.{ts,tsx,js,jsx,rs,py,sh} : Never add any form of comments to code (single-line //, multi-line /* */, documentation ///, //!, /** */, or any other comment syntax). Code must be self-explanatory through naming, types, and structure.
Applied to files:
apps/desktop/src-tauri/src/recording.rs
📚 Learning: 2025-11-29T04:31:23.896Z
Learnt from: CR
Repo: CapSoftware/Cap PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T04:31:23.896Z
Learning: Applies to **/*.{ts,tsx,js,jsx,rs} : Never add comments to code in any language (no `//`, `/* */`, `///`, `//!`, `#`, etc.). Code must be self-explanatory through naming, types, and structure
Applied to files:
apps/desktop/src-tauri/src/recording.rs
🧬 Code graph analysis (1)
apps/desktop/src-tauri/src/recording.rs (1)
crates/utils/src/lib.rs (2)
moment_format_to_chrono(186-218)ensure_unique_filename(52-58)
⏰ 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). (1)
- GitHub Check: Clippy (aarch64-apple-darwin, macos-latest)
🔇 Additional comments (4)
apps/desktop/src-tauri/src/recording.rs (4)
26-32: LGTM on new imports.The imports appropriately support the template-based project naming feature:
lazy_staticfor compiled regex/Aho-Corasick patterns,moment_format_to_chronofor date format conversion, andCowfor efficient string handling.
446-468: Well-structured project name generation and path handling.The implementation correctly:
- Retrieves the custom template from settings with a sensible fallback
- Handles missing target titles with "Unknown"
- Sanitizes the filename to prevent path issues
- Ensures uniqueness to avoid directory collisions
- Properly constructs the recording directory path
The explicit colon-to-period replacement on Line 460 before sanitization ensures consistent Windows compatibility, though
sanitize_filenameshould also handle this.
486-507: Good alignment of video upload name with project name.Passing the formatted
project_nametocreate_or_get_video(Line 490) ensures the backend video record uses a meaningful, user-customizable name instead of a timestamp or UUID, improving consistency between local and remote naming.
529-555: Correct propagation of project name through metadata.Setting
pretty_nametoproject_name(Line 532) ensures the recording metadata reflects the formatted, customizable project name. This properly integrates the new naming system with the project metadata structure.
| let result = TIME_REGEX.replace_all(&result, |caps: ®ex::Captures| { | ||
| datetime | ||
| .format( | ||
| &caps | ||
| .get(1) | ||
| .map(|m| m.as_str()) | ||
| .map(moment_format_to_chrono) | ||
| .unwrap_or(Cow::Borrowed("%I:%M %p")), | ||
| ) | ||
| .to_string() | ||
| }); |
There was a problem hiding this comment.
Time format produces zero-padded hours on Windows.
The default time format %I:%M %p on Line 411 produces zero-padded hours like "03:23 PM" on all platforms. If documentation or UI examples show non-padded hours like "3:23 PM", there's a mismatch. The Unix-specific %-I flag for non-padded hours doesn't work on Windows, creating a cross-platform inconsistency.
Options to resolve:
- Accept zero-padding (simplest): Update any documentation/examples to show "03:23 PM"
- Platform-specific formats:
+#[cfg(not(windows))]
+const DEFAULT_TIME_FORMAT: &str = "%-I:%M %p";
+#[cfg(windows)]
+const DEFAULT_TIME_FORMAT: &str = "%I:%M %p";
+
let result = TIME_REGEX.replace_all(&result, |caps: ®ex::Captures| {
datetime
.format(
&caps
.get(1)
.map(|m| m.as_str())
.map(moment_format_to_chrono)
- .unwrap_or(Cow::Borrowed("%I:%M %p")),
+ .unwrap_or(Cow::Borrowed(DEFAULT_TIME_FORMAT)),
)
.to_string()
});- Post-process to remove leading zeros (more complex, affects all platforms)
Would you like me to implement one of these solutions?
There was a problem hiding this comment.
Chrono's docs don't mention anything about it being platform specific. I'm pretty sure it's locale based. The frontend calls the same format function for showing examples so it's always showing the true result.
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
rust chrono strftime format flags platform specific
💡 Result:
Short answer: Chrono provides its own strftime-like implementation with a stable, cross‑platform set of format specifiers (so you generally do not get the platform-dependent oddities of C's strftime). A few notes/compatibility points:
- Chrono documents the complete set of supported specifiers and padding modifiers (%, %- , %_ , %0 , etc.) and implements them consistently across platforms. [1][2]
- Some semantics intentionally differ from C/OS implementations: e.g. chrono’s %s (seconds since epoch) and fractional‑seconds (%f / %.3f / %.6f / %.9f) have chrono‑defined padding/behavior; %+ is given a stable ISO‑like form in chrono (unlike platform/locale C implementations) but is advised to be avoided if you need exact control. [1]
- Locale‑dependent specifiers (like %x, %X, %c, %r) are only localized when you opt into chrono’s locale feature (unstable‑locales); otherwise chrono uses its own defaults. [1][3]
- If you need exact parity with a C library’s strftime (glibc, musl, Windows CRT), don’t assume full correspondence — consult chrono’s specifiers in chrono::format::strftime or the source. [1][4]
Sources:
- Chrono strftime docs and source (specifiers, modifiers, notes about %+, %s, %f and locales). [1][3][4]
- C/C++ strftime platform differences reference (for contrast). [2]
@ItsEeleeya You're absolutely right, and thank you for the correction! I confused chrono with C's strftime, which does have platform-specific limitations.
Chrono provides its own cross-platform implementation with stable format specifiers that work consistently across all platforms, including padding modifiers like %-. The behavior is locale-based (when the unstable-locales feature is enabled), not platform-specific.
Since the frontend uses the same format_project_name function for examples, there's complete consistency between what users see in the UI and the actual generated filenames. The current implementation is correct as-is.
My apologies for the confusion!
1 participant
This PR adds a new recording setting: Default project name which allows for customizing the default project and file name using a custom template with placeholders.
There are several place holders for different info about a recording, and a custom date placeholder which accepts a moment format.
Renaming the project through the editor still has the same behavior, only the pretty name changes, not the actual name of the file.
There's a new module
update_project_nameswhich updates the name of all previous project files to use their pretty name instead. Once done, it addsuuid_projects_migratedto the data store.Other changes:
Note: I'm not sure how to format the Cargo.toml file, I don't even know why it changed.
Summary by CodeRabbit
New Features
Improvements
Tweaks
Additions
✏️ Tip: You can customize this high-level summary in your review settings.