Skip to content

Add CLI to open file #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TeamDman wants to merge 1 commit into from
Closed

Add CLI to open file #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/app/thoth_app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ pub struct ThothApp {
}

impl ThothApp {
/// Create a new ThothApp with loaded settings
pub fn new(settings: settings::Settings) -> Self {
/// Create a new ThothApp with loaded settings and optional initial file to open
pub fn new(settings: settings::Settings, initial_file: Option<std::path::PathBuf>) -> Self {
// Load persistent state (recent files, sidebar width, etc.)
let persistent_state = PersistentState::default();

Expand All @@ -43,6 +43,12 @@ impl ThothApp {
window_state.sidebar_expanded = persistent_state.get_sidebar_expanded();
}

// If an initial file was provided via CLI, set it so central panel will attempt to open it on first render
if let Some(path) = initial_file {
window_state.file_path = Some(path);
window_state.error = None;
}
Comment on lines +46 to +50
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initial file handling logic directly sets the file_path without considering whether file_type needs to be determined. While the CentralPanel's file opening logic handles file type detection (lines 75-93 in central_panel.rs), it might be more explicit to document this dependency or ensure file_type is left as default when setting an initial file path. The current implementation relies on implicit default behavior which may not be immediately clear to future maintainers.

Copilot uses AI. Check for mistakes.

// Initialize navigation history with configured size
window_state.navigation_history =
state::NavigationHistory::with_capacity(settings.performance.navigation_history_size);
Expand Down
16 changes: 12 additions & 4 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,17 @@ fn main() -> Result<()> {
// Note: Settings mode is no longer supported
// Settings are now opened via viewport mode from the main application
//
// // Check if launched in settings mode
// let args: Vec<String> = std::env::args().collect();
// let is_settings_mode = args.iter().any(|arg| arg == "--settings");
// Parse simple CLI flags: --help / -h and optional FILE arg
let mut cli_file: Option<std::path::PathBuf> = None;
for arg in std::env::args_os().skip(1) {
if arg == std::ffi::OsString::from("--help") || arg == std::ffi::OsString::from("-h") {
println!("Thoth — JSON & NDJSON Viewer\n\nUsage: thoth [OPTIONS] [FILE]\n\nOptions:\n -h, --help Show this help message\n\nIf a FILE is supplied, Thoth will open it on startup.");
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help message format is inconsistent with standard CLI conventions. The entire message is on a single line making it harder to read and maintain. Additionally, the em dash character (—) may not render properly in all terminal environments. Consider breaking this into multiple lines and using a regular hyphen for better compatibility.

Suggested change
println!("Thoth — JSON & NDJSON Viewer\n\nUsage: thoth [OPTIONS] [FILE]\n\nOptions:\n -h, --help Show this help message\n\nIf a FILE is supplied, Thoth will open it on startup.");
println!(
"Thoth - JSON & NDJSON Viewer\n\
\n\
Usage: thoth [OPTIONS] [FILE]\n\
\n\
Options:\n\
-h, --help Show this help message\n\
\n\
If a FILE is supplied, Thoth will open it on startup."
);

Copilot uses AI. Check for mistakes.
return Ok(());
}
// First non-flag arg is treated as file path
cli_file = Some(std::path::PathBuf::from(arg));
break;
}
Comment on lines +42 to +52
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI file path argument lacks test coverage. Given that the codebase has comprehensive tests for other components, this new CLI functionality should have corresponding tests to verify proper argument parsing, help message output, and file path handling. Consider adding integration tests that validate the CLI argument parsing behavior.

Copilot uses AI. Check for mistakes.

Comment on lines +44 to 53
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI argument parsing logic has a flaw: if multiple arguments are provided, only the first non-flag argument is captured as the file path, but any subsequent arguments are silently ignored without validation. This could confuse users who pass multiple file paths expecting them all to be opened or rejected with an error. Consider adding validation to detect and warn about unexpected additional arguments.

Suggested change
for arg in std::env::args_os().skip(1) {
if arg == std::ffi::OsString::from("--help") || arg == std::ffi::OsString::from("-h") {
println!("Thoth — JSON & NDJSON Viewer\n\nUsage: thoth [OPTIONS] [FILE]\n\nOptions:\n -h, --help Show this help message\n\nIf a FILE is supplied, Thoth will open it on startup.");
return Ok(());
}
// First non-flag arg is treated as file path
cli_file = Some(std::path::PathBuf::from(arg));
break;
}
let mut extra_args: Vec<std::ffi::OsString> = Vec::new();
for arg in std::env::args_os().skip(1) {
if arg == std::ffi::OsString::from("--help") || arg == std::ffi::OsString::from("-h") {
println!("Thoth — JSON & NDJSON Viewer\n\nUsage: thoth [OPTIONS] [FILE]\n\nOptions:\n -h, --help Show this help message\n\nIf a FILE is supplied, Thoth will open it on startup.");
return Ok(());
}
// First non-flag arg is treated as file path; any additional args are rejected
if cli_file.is_none() {
cli_file = Some(std::path::PathBuf::from(&arg));
} else {
extra_args.push(arg);
}
}
if !extra_args.is_empty() {
eprintln!("Error: unexpected additional argument(s): {:?}", extra_args);
eprintln!(
"Usage: thoth [OPTIONS] [FILE]\n\nOptions:\n -h, --help Show this help message\n\nIf a FILE is supplied, Thoth will open it on startup."
);
return Err("Unexpected additional CLI arguments".into());
}

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CLI parsing does not validate whether the provided file path exists or is accessible before passing it to the application. While the CentralPanel will eventually handle file opening errors, early validation here would provide a better user experience by giving immediate feedback if the file doesn't exist, rather than launching the GUI only to show an error. Consider adding a basic existence check and providing a meaningful error message on the command line.

Suggested change
// If a CLI file was provided, validate that it exists before launching the GUI
if let Some(ref path) = cli_file {
if !path.exists() {
eprintln!(
"Error: File '{}' does not exist or is not accessible.",
path.to_string_lossy()
);
return Err("CLI file path does not exist".into());
}
}

Copilot uses AI. Check for mistakes.
// Load settings first
let settings = settings::Settings::load().unwrap_or_else(|e| {
Expand Down Expand Up @@ -84,7 +92,7 @@ fn main() -> Result<()> {
egui_phosphor::add_to_fonts(&mut fonts, egui_phosphor::Variant::Regular);
cc.egui_ctx.set_fonts(fonts);

Ok(Box::new(app::ThothApp::new(settings)))
Ok(Box::new(app::ThothApp::new(settings, cli_file)))
}),
);

Expand Down
Loading