feat: File Association Support (Issue #56)#57
Conversation
- Parse file path from command-line arguments - Validate and sanitize file paths (prevent directory traversal) - Support .json, .ndjson, .jsonl, .geojson extensions - Pass file path to ThothApp on initialization - Add comprehensive unit tests for argument parsing Implements Phase 1 of Issue #56: File Association Tests: 6 new tests added, all 140 tests passing
Platform-specific configuration for file associations: macOS (.app bundle): - Register .json, .ndjson, .jsonl, .geojson file types - Configure CFBundleDocumentTypes via cargo-packager - Enable 'Open With' menu and default app selection Windows (MSI installer): - Register file associations in Windows Registry - Set Thoth icon for associated file types - Enable 'Open With' context menu integration Linux (.deb package): - Create .desktop file with MimeType declarations - Support application/json, application/x-ndjson, application/geo+json - Enable default app selection in file managers Implements Phase 2 of Issue #56: File Association All tests passing (140/140)
Added FILE_ASSOCIATIONS.md with: - Command-line usage examples - Platform-specific setup instructions (macOS, Windows, Linux) - Setting Thoth as default app for JSON files - Troubleshooting common issues - Developer guide for building with file associations Updated README.md: - Added 'Opening Files' section with three methods - Added command-line usage examples - Added link to FILE_ASSOCIATIONS.md in documentation section Part of Issue #56: File Association
There was a problem hiding this comment.
Pull request overview
This PR implements comprehensive file association support, enabling users to open JSON files directly from file managers, command lines, or by setting Thoth as the default JSON viewer. The implementation includes command-line argument parsing with validation, platform-specific configuration for macOS, Windows, and Linux, and extensive documentation.
Key Changes:
- Added command-line argument parsing with path validation and JSON extension verification
- Configured platform-specific file associations via cargo-packager metadata
- Created comprehensive user and developer documentation
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.rs | Added parse_file_argument() function with validation logic and 6 unit tests; updated main() to parse arguments and pass file path to ThothApp |
| src/app/thoth_app.rs | Modified ThothApp::new() to accept optional file path and set it in window state for loading |
| docs/FILE_ASSOCIATIONS.md | Created comprehensive guide covering command-line usage, platform-specific setup, troubleshooting, and developer instructions |
| assets/thoth.desktop | Added Linux desktop entry file declaring MIME type support for JSON files |
| README.md | Updated usage section with examples for opening files via command line and file manager |
| Cargo.toml | Added platform-specific file association configurations for macOS, Windows, and Linux |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Parse command-line arguments to extract file path | ||
| fn parse_file_argument(args: &[String]) -> Result<Option<PathBuf>> { | ||
| // Skip first argument (executable name) | ||
| if args.len() < 2 { | ||
| return Ok(None); | ||
| } | ||
|
|
There was a problem hiding this comment.
The function only processes the first file argument. Consider documenting this limitation or handling multiple file arguments to match typical file manager behavior where users can select and open multiple files at once.
| /// Parse command-line arguments to extract file path | |
| fn parse_file_argument(args: &[String]) -> Result<Option<PathBuf>> { | |
| // Skip first argument (executable name) | |
| if args.len() < 2 { | |
| return Ok(None); | |
| } | |
| /// Parse command-line arguments to extract a single file path. | |
| /// | |
| /// Only the first argument after the executable name is considered a file path. | |
| /// Any additional arguments are ignored. | |
| fn parse_file_argument(args: &[String]) -> Result<Option<PathBuf>> { | |
| // Skip first argument (executable name) | |
| if args.len() < 2 { | |
| return Ok(None); | |
| } | |
| if args.len() > 2 { | |
| eprintln!( | |
| "Warning: multiple file arguments detected; only the first file will be opened" | |
| ); | |
| } |
| file_path_str | ||
| ); | ||
| // Allow opening anyway - user might know what they're doing | ||
| } |
There was a problem hiding this comment.
The file extension validation is silent when a file has no extension at all. Consider adding a warning message for files without extensions, similar to the warning for non-JSON extensions, to provide consistent user feedback.
| } | |
| } | |
| } else { | |
| eprintln!( | |
| "Warning: File '{}' has no extension; expected a JSON file", | |
| file_path_str | |
| ); | |
| // Allow opening anyway - user might know what they're doing |
| **macOS:** | ||
| Run this command to rebuild the Launch Services database: | ||
| ```bash | ||
| /System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister -kill -r -domain local -domain system -domain user |
There was a problem hiding this comment.
This extremely long command is difficult to read and may wrap awkwardly in documentation. Consider breaking it into multiple lines using backslashes for better readability, or providing it as a script snippet.
| /System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister -kill -r -domain local -domain system -domain user | |
| /System/Library/Frameworks/CoreServices.framework/Frameworks/LaunchServices.framework/Support/lsregister \ | |
| -kill -r \ | |
| -domain local \ | |
| -domain system \ | |
| -domain user |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
==========================================
+ Coverage 26.87% 27.22% +0.34%
==========================================
Files 58 59 +1
Lines 5016 5154 +138
==========================================
+ Hits 1348 1403 +55
- Misses 3668 3751 +83 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Adds ability to register/unregister Thoth in system PATH from Settings > Advanced tab. Features: - Platform-specific PATH registration (macOS, Windows, Linux) - Check if 'thoth' command is available in PATH - Add/Remove buttons in Advanced settings tab - Real-time status indicator (✓ Available / ✗ Not available) - Comprehensive error handling with user-friendly messages Implementation: - Created path_registry module with platform-specific implementations - macOS/Linux: Modifies shell profile files (.zshrc, .bashrc, .profile) - Windows: Uses PowerShell to modify User PATH environment variable - Added PathRegistryError to error handling system - UI in Advanced tab with themed buttons and status display Technical details: - Uses 'which' crate to check PATH availability - Safe path handling with canonicalization - Prevents duplicate PATH entries - Proper cleanup when unregistering Tests: - 3 new unit tests for path_registry module - All 140 existing tests passing Part of Issue #56 file association work
Replace ✓ and ✗ text symbols with CHECK_CIRCLE and X_CIRCLE icons from egui-phosphor for better visual consistency with the rest of the UI.
- Changed 'Add to PATH' button color from info to overlay1 for better contrast - Added ctx.request_repaint() after PATH events to update UI immediately - Status indicator now updates right away after registration/unregistration
Updated note to explicitly mention restarting Thoth itself, not just the terminal, since the running process has the old PATH environment.
- Replace flatten() with map_while(|r| r.ok()) to handle IO errors properly - Replace filter_map(|l| l.ok()) with map_while(|r| r.ok()) for better error handling - Prevents potential infinite loops on repeated IO errors All 137 tests passing
Moved path_registry.rs to platform/path_registry.rs for better code organization. Platform-specific implementations should live in the platform module alongside other platform abstractions. Changes: - Moved src/path_registry.rs → src/platform/path_registry.rs - Updated imports in lib.rs, main.rs, thoth_app.rs, and settings_dialog - All references now use crate::platform::path_registry::* All 140 tests passing, no clippy warnings
Added handling for RegisterInPath and UnregisterFromPath events in the profiling test to satisfy exhaustive match checking when running with -D warnings. Passes cargo clippy --all-targets --all-features -- -D warnings
Changed imports from macos-only to any(target_os = "macos", target_os = "linux") for BufReader, OpenOptions, and related types. These are needed for both macOS and Linux implementations. Fixes CI build error on Linux targets.
… warning Changed the Linux implementation to use the same pattern as macOS: reader.lines().map_while(|r| r.ok()) instead of if let Ok(line). Passes strict clippy with -D warnings.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Overview
Implements file association support so users can open JSON files directly from their file manager or command line.
Changes
Phase 1: Command-Line Arguments (commit d864d27)
parse_file_argument()function to parse and validate file pathsThothApp::new()to acceptOption<PathBuf>for initial filePhase 2: Platform Configuration (commit 0907384)
macOS (.app bundle):
[package.metadata.packager.macos]configurationWindows (MSI installer):
[package.metadata.packager.windows]configurationLinux (.deb package):
assets/thoth.desktopfilePhase 3: Documentation (commit 8d6e83a)
docs/FILE_ASSOCIATIONS.mdguideTesting
All 140 tests pass:
Usage Examples
Closes
Closes #56
Related Documentation