Skip to content

Commit 0740ea6

Browse files
TimelordUKclaude
andcommitted
refactor: Create argument parsing context object (Python argparse style)
Replace verbose individual argument parsing with a structured approach using a NonInteractiveArgs context object. This follows the pattern of Python's argparse and significantly improves maintainability. Changes: - Added NonInteractiveArgs struct to hold all parsed arguments - Created parse_non_interactive_args() function to parse into context - Updated handle_non_interactive_query() to accept the context object - Simplified main() from 84 individual arg parsing lines to single call Benefits: - Cleaner separation of concerns (parsing vs execution) - Easier to add new arguments (just add to struct) - More testable (can mock the struct) - Reduced main() complexity Progress: - main.rs: 989 → 984 lines (5 lines removed, net positive change) - Total reduction: 983 lines removed from original 1967 (49%) - Much cleaner argument handling pattern Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
1 parent cbc62d9 commit 0740ea6

1 file changed

Lines changed: 128 additions & 133 deletions

File tree

src/main.rs

Lines changed: 128 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -376,10 +376,8 @@ fn execute_query(client: &ApiClient, query: &str) -> Result<(), Box<dyn std::err
376376
}
377377
}
378378

379-
/// Handle non-interactive query mode
380-
/// Executes queries from command line or file and outputs results
381-
fn handle_non_interactive_query(
382-
args: &[String],
379+
/// Parsed command-line arguments for non-interactive mode
380+
struct NonInteractiveArgs {
383381
query_arg: Option<String>,
384382
query_file_arg: Option<String>,
385383
output_format_arg: String,
@@ -397,14 +395,110 @@ fn handle_non_interactive_query(
397395
expand_star_arg: bool,
398396
extract_cte_arg: Option<String>,
399397
query_at_position_arg: Option<String>,
398+
}
399+
400+
/// Parse command-line arguments into a structured context object
401+
fn parse_non_interactive_args(args: &[String]) -> NonInteractiveArgs {
402+
NonInteractiveArgs {
403+
query_arg: args
404+
.iter()
405+
.position(|arg| arg == "-q" || arg == "--query")
406+
.and_then(|pos| args.get(pos + 1))
407+
.map(std::string::ToString::to_string),
408+
409+
query_file_arg: args
410+
.iter()
411+
.position(|arg| arg == "-f" || arg == "--query-file")
412+
.and_then(|pos| args.get(pos + 1))
413+
.map(std::string::ToString::to_string),
414+
415+
output_format_arg: args
416+
.iter()
417+
.position(|arg| arg == "-o" || arg == "--output")
418+
.and_then(|pos| args.get(pos + 1))
419+
.map_or_else(|| "csv".to_string(), std::string::ToString::to_string),
420+
421+
output_file_arg: args
422+
.iter()
423+
.position(|arg| arg == "-O" || arg == "--output-file")
424+
.and_then(|pos| args.get(pos + 1))
425+
.map(std::string::ToString::to_string),
426+
427+
table_style_arg: args
428+
.iter()
429+
.position(|arg| arg == "--table-style")
430+
.and_then(|pos| args.get(pos + 1))
431+
.map_or_else(|| "default".to_string(), std::string::ToString::to_string),
432+
433+
query_plan_arg: args
434+
.iter()
435+
.any(|arg| arg == "--query-plan" || arg == "--query_plan"),
436+
437+
show_work_units_arg: args
438+
.iter()
439+
.any(|arg| arg == "--show-work-units" || arg == "--show_work_units"),
440+
441+
lift_in_arg: args
442+
.iter()
443+
.any(|arg| arg == "--check-in-lifting" || arg == "--check_in_lifting"),
444+
445+
analyze_query_arg: args
446+
.iter()
447+
.any(|arg| arg == "--analyze-query" || arg == "--analyze_query"),
448+
449+
expand_star_arg: args
450+
.iter()
451+
.any(|arg| arg == "--expand-star" || arg == "--expand_star"),
452+
453+
extract_cte_arg: args
454+
.iter()
455+
.position(|arg| arg == "--extract-cte" || arg == "--extract_cte")
456+
.and_then(|pos| args.get(pos + 1))
457+
.map(std::string::ToString::to_string),
458+
459+
query_at_position_arg: args
460+
.iter()
461+
.position(|arg| arg == "--query-at-position" || arg == "--query_at_position")
462+
.and_then(|pos| args.get(pos + 1))
463+
.map(std::string::ToString::to_string),
464+
465+
execution_plan_arg: args
466+
.iter()
467+
.any(|arg| arg == "--execution-plan" || arg == "--execution_plan"),
468+
469+
cte_info_arg: args
470+
.iter()
471+
.any(|arg| arg == "--cte-info" || arg == "--cte-json"),
472+
473+
rewrite_analysis_arg: args
474+
.iter()
475+
.any(|arg| arg == "--analyze-rewrite" || arg == "--rewrite-analysis"),
476+
477+
debug_arg: args
478+
.iter()
479+
.any(|arg| arg == "--debug" || arg == "--debug-trace"),
480+
481+
limit_arg: args
482+
.iter()
483+
.position(|arg| arg == "-l" || arg == "--limit")
484+
.and_then(|pos| args.get(pos + 1))
485+
.and_then(|s| s.parse::<usize>().ok()),
486+
}
487+
}
488+
489+
/// Handle non-interactive query mode
490+
/// Executes queries from command line or file and outputs results
491+
fn handle_non_interactive_query(
492+
args: &[String],
493+
parsed_args: NonInteractiveArgs,
400494
) -> io::Result<()> {
401495
// Read query from file if specified
402-
let query_file = query_file_arg.clone();
496+
let query_file = parsed_args.query_file_arg.clone();
403497
let query = if let Some(file) = &query_file {
404498
std::fs::read_to_string(file)
405499
.map_err(|e| io::Error::other(format!("Failed to read query file {file}: {e}")))?
406500
} else {
407-
query_arg.unwrap()
501+
parsed_args.query_arg.unwrap()
408502
};
409503

410504
// Check if this is a multi-statement script (contains GO separator)
@@ -440,10 +534,10 @@ fn handle_non_interactive_query(
440534
.unwrap_or(100);
441535

442536
// Handle query analysis commands (for IDE/plugin integration)
443-
if analyze_query_arg
444-
|| expand_star_arg
445-
|| extract_cte_arg.is_some()
446-
|| query_at_position_arg.is_some()
537+
if parsed_args.analyze_query_arg
538+
|| parsed_args.expand_star_arg
539+
|| parsed_args.extract_cte_arg.is_some()
540+
|| parsed_args.query_at_position_arg.is_some()
447541
{
448542
use sql_cli::analysis;
449543
use sql_cli::sql::recursive_parser::Parser;
@@ -455,7 +549,7 @@ fn handle_non_interactive_query(
455549
})?;
456550

457551
// Handle different analysis commands
458-
if analyze_query_arg {
552+
if parsed_args.analyze_query_arg {
459553
let analysis = analysis::analyze_query(&ast, &query);
460554
println!(
461555
"{}",
@@ -464,7 +558,7 @@ fn handle_non_interactive_query(
464558
return Ok(());
465559
}
466560

467-
if expand_star_arg {
561+
if parsed_args.expand_star_arg {
468562
// TODO: Implement column expansion
469563
// This requires loading data or executing CTEs to get schema
470564
eprintln!("--expand-star: Not yet implemented");
@@ -475,7 +569,7 @@ fn handle_non_interactive_query(
475569
));
476570
}
477571

478-
if let Some(cte_name) = extract_cte_arg {
572+
if let Some(cte_name) = parsed_args.extract_cte_arg {
479573
if let Some(cte_query) = analysis::extract_cte(&ast, &cte_name) {
480574
println!("{}", cte_query);
481575
return Ok(());
@@ -487,7 +581,7 @@ fn handle_non_interactive_query(
487581
}
488582
}
489583

490-
if let Some(position) = query_at_position_arg {
584+
if let Some(position) = parsed_args.query_at_position_arg {
491585
let parts: Vec<&str> = position.split(':').collect();
492586
if parts.len() != 2 {
493587
return Err(io::Error::new(
@@ -522,23 +616,25 @@ fn handle_non_interactive_query(
522616
let config = sql_cli::non_interactive::NonInteractiveConfig {
523617
data_file,
524618
query,
525-
output_format: sql_cli::non_interactive::OutputFormat::from_str(&output_format_arg)
526-
.map_err(io::Error::other)?,
527-
output_file: output_file_arg,
619+
output_format: sql_cli::non_interactive::OutputFormat::from_str(
620+
&parsed_args.output_format_arg,
621+
)
622+
.map_err(io::Error::other)?,
623+
output_file: parsed_args.output_file_arg,
528624
case_insensitive: args.contains(&"--case-insensitive".to_string()),
529625
auto_hide_empty: args.contains(&"--auto-hide-empty".to_string()),
530-
limit: limit_arg,
531-
query_plan: query_plan_arg,
532-
show_work_units: show_work_units_arg,
533-
execution_plan: execution_plan_arg,
534-
cte_info: cte_info_arg,
535-
rewrite_analysis: rewrite_analysis_arg,
536-
lift_in_expressions: lift_in_arg,
537-
script_file: query_file_arg.clone(),
538-
debug_trace: debug_arg,
626+
limit: parsed_args.limit_arg,
627+
query_plan: parsed_args.query_plan_arg,
628+
show_work_units: parsed_args.show_work_units_arg,
629+
execution_plan: parsed_args.execution_plan_arg,
630+
cte_info: parsed_args.cte_info_arg,
631+
rewrite_analysis: parsed_args.rewrite_analysis_arg,
632+
lift_in_expressions: parsed_args.lift_in_arg,
633+
script_file: parsed_args.query_file_arg.clone(),
634+
debug_trace: parsed_args.debug_arg,
539635
max_col_width,
540636
col_sample_rows,
541-
table_style: sql_cli::non_interactive::TableStyle::from_str(&table_style_arg)
637+
table_style: sql_cli::non_interactive::TableStyle::from_str(&parsed_args.table_style_arg)
542638
.map_err(io::Error::other)?,
543639
};
544640

@@ -594,98 +690,16 @@ fn main() -> io::Result<()> {
594690
return result;
595691
}
596692

597-
// Check for non-interactive query mode
598-
let query_arg = args
599-
.iter()
600-
.position(|arg| arg == "-q" || arg == "--query")
601-
.and_then(|pos| args.get(pos + 1))
602-
.map(std::string::ToString::to_string);
603-
604-
let query_file_arg = args
605-
.iter()
606-
.position(|arg| arg == "-f" || arg == "--query-file")
607-
.and_then(|pos| args.get(pos + 1))
608-
.map(std::string::ToString::to_string);
609-
610-
let output_format_arg = args
611-
.iter()
612-
.position(|arg| arg == "-o" || arg == "--output")
613-
.and_then(|pos| args.get(pos + 1))
614-
.map_or_else(|| "csv".to_string(), std::string::ToString::to_string);
615-
616-
let output_file_arg = args
617-
.iter()
618-
.position(|arg| arg == "-O" || arg == "--output-file")
619-
.and_then(|pos| args.get(pos + 1))
620-
.map(std::string::ToString::to_string);
621-
622-
let table_style_arg = args
623-
.iter()
624-
.position(|arg| arg == "--table-style")
625-
.and_then(|pos| args.get(pos + 1))
626-
.map_or_else(|| "default".to_string(), std::string::ToString::to_string);
627-
628-
let query_plan_arg = args
629-
.iter()
630-
.any(|arg| arg == "--query-plan" || arg == "--query_plan");
631-
632-
let show_work_units_arg = args
633-
.iter()
634-
.any(|arg| arg == "--show-work-units" || arg == "--show_work_units");
635-
636-
let lift_in_arg = args
637-
.iter()
638-
.any(|arg| arg == "--check-in-lifting" || arg == "--check_in_lifting");
639-
640-
// Query analysis flags (for IDE/plugin integration)
641-
let analyze_query_arg = args
642-
.iter()
643-
.any(|arg| arg == "--analyze-query" || arg == "--analyze_query");
644-
645-
let expand_star_arg = args
646-
.iter()
647-
.any(|arg| arg == "--expand-star" || arg == "--expand_star");
648-
649-
let extract_cte_arg = args
650-
.iter()
651-
.position(|arg| arg == "--extract-cte" || arg == "--extract_cte")
652-
.and_then(|pos| args.get(pos + 1))
653-
.map(std::string::ToString::to_string);
654-
655-
let query_at_position_arg = args
656-
.iter()
657-
.position(|arg| arg == "--query-at-position" || arg == "--query_at_position")
658-
.and_then(|pos| args.get(pos + 1))
659-
.map(std::string::ToString::to_string);
660-
661-
let execution_plan_arg = args
662-
.iter()
663-
.any(|arg| arg == "--execution-plan" || arg == "--execution_plan");
664-
665-
let cte_info_arg = args
666-
.iter()
667-
.any(|arg| arg == "--cte-info" || arg == "--cte-json");
668-
669-
let rewrite_analysis_arg = args
670-
.iter()
671-
.any(|arg| arg == "--analyze-rewrite" || arg == "--rewrite-analysis");
672-
673-
let debug_arg = args
674-
.iter()
675-
.any(|arg| arg == "--debug" || arg == "--debug-trace");
676-
677-
let limit_arg = args
678-
.iter()
679-
.position(|arg| arg == "-l" || arg == "--limit")
680-
.and_then(|pos| args.get(pos + 1))
681-
.and_then(|s| s.parse::<usize>().ok());
693+
// Parse non-interactive arguments into a structured context
694+
let parsed_args = parse_non_interactive_args(&args);
682695

683696
// Initialize unified logging (tracing + dual logging) for both modes
684697
// Do this before non-interactive mode so we get debug logs
685698
sql_cli::utils::logging::init_tracing_with_dual_logging();
686699

687700
// Check if running in non-interactive mode
688-
let is_non_interactive = query_arg.is_some() || query_file_arg.is_some();
701+
let is_non_interactive =
702+
parsed_args.query_arg.is_some() || parsed_args.query_file_arg.is_some();
689703

690704
// Only show log path in interactive mode or if debug logging is enabled
691705
if !is_non_interactive || std::env::var("RUST_LOG").is_ok() {
@@ -701,26 +715,7 @@ fn main() -> io::Result<()> {
701715

702716
// If we have a query, run in non-interactive mode
703717
if is_non_interactive {
704-
return handle_non_interactive_query(
705-
&args,
706-
query_arg,
707-
query_file_arg,
708-
output_format_arg,
709-
output_file_arg,
710-
table_style_arg,
711-
query_plan_arg,
712-
show_work_units_arg,
713-
execution_plan_arg,
714-
cte_info_arg,
715-
rewrite_analysis_arg,
716-
lift_in_arg,
717-
debug_arg,
718-
limit_arg,
719-
analyze_query_arg,
720-
expand_star_arg,
721-
extract_cte_arg,
722-
query_at_position_arg,
723-
);
718+
return handle_non_interactive_query(&args, parsed_args);
724719
}
725720

726721
// Check for config initialization

0 commit comments

Comments
 (0)