-
Notifications
You must be signed in to change notification settings - Fork 0
Implement comprehensive CLI with summary feature #18
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.
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
Conversation
Replace basic command-line parsing with clap-based robust CLI and add beautiful GEDCOM summary output using tabled. Features added: - --version flag: Display application version - --dump flag: Output entire GEDCOM structure (replaces default behavior) - --verbose flag: Show detailed encoding warnings - --summary flag: Display formatted summary (new default behavior) - --home-xref flag: Specify which individual to use for genealogy analysis Summary output includes: - Source system information from GEDCOM header - GEDCOM version and form - Statistics table with counts of all record types: * Individuals, Families, Sources, Repositories * Notes, Multimedia, Submitters - Validation warnings count (detailed list with --verbose) - Home individual information: * Name, XREF, birth/death dates and places * Genealogy depth (ancestor/descendant generations) * Immediate family counts (parents, siblings, spouses, children) * Extended family counts (total ancestors/descendants up to 10 gens) Implementation details: - Uses clap 4.5 with derive feature for ergonomic CLI - Uses tabled 0.16 for beautiful table formatting - Added calculate_max_generations_ancestors/descendants functions using breadth-first search to find maximum genealogy depth - Leverages existing Gedcom API methods (get_parents, get_children, etc.) - Summary is now default behavior, --dump preserves debug output Resolves TODO in src/main.rs to create pretty summary output.
- Add term_size dependency for terminal width detection - Implement responsive layout: wide (>=120 cols) vs narrow (<120 cols) - Wide layout displays file summary and home individual side-by-side - Narrow layout displays them sequentially (existing behavior) - Refactor print_summary into modular helper functions for maintainability - Fix unused import warning in src/types/mod.rs (removed std::fs) - Fix useless comparison warnings in src/main.rs test (usize >= 0) - All 277 tests pass, clippy clean, properly formatted
- Change GEDCOM Form display from Debug format to just the name field
- Before: 'GEDCOM Form: Form { name: Some("LINEAGE-LINKED"), version: Some("5.5.5") }'
- After: 'GEDCOM Form: LINEAGE-LINKED'
- Updated both print_file_info() and collect_file_info() functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive CLI interface using clap and adds a formatted summary feature using tabled, replacing the debug output with a user-friendly summary display. The changes transform the binary from a simple parser that dumps raw GEDCOM data into a sophisticated analysis tool with multiple output modes and genealogical insights.
Key Changes:
- Added CLI argument parsing with
clapsupporting multiple flags (--dump,--summary,--verbose,--home-xref) - Implemented formatted summary output with statistics tables and genealogical analysis (ancestor/descendant generation calculations)
- Changed default behavior from debug dump to formatted summary display
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| Cargo.toml | Added dependencies: clap (CLI parsing), tabled (table formatting), term_size (terminal width detection) |
| src/main.rs | Complete rewrite: replaced simple argument parsing with clap, added summary output functions, implemented BFS-based genealogy depth calculations, added wide/narrow layout support |
| src/types/mod.rs | Removed unused std::fs import from test module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main.rs
Outdated
| #[arg(short, long, default_value_t = true)] | ||
| summary: bool, |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The summary flag with default_value_t = true creates confusing CLI behavior. When both --dump and --summary are specified, both would be true, but the code prioritizes --dump (line 65 checks dump first). Consider making these mutually exclusive using clap's #[group] attribute or removing the summary flag entirely since it's the default behavior.
src/main.rs
Outdated
| let spouses = gedcom.get_spouses(individual); | ||
|
|
||
| println!("Immediate Family:"); | ||
| println!(" Parents: {}", if parents.is_empty() { 0 } else { 2 }); |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic assumes exactly 2 parents when any parents exist, but get_parents() can return tuples with None values (e.g., (Some(father), None)). The count should be based on the actual number of non-None parents in the result, not a binary check. Use parents.iter().filter(|(f, m)| f.is_some() || m.is_some()).count() or count the individual Some values.
| println!(" Parents: {}", if parents.is_empty() { 0 } else { 2 }); | |
| let parent_count = parents.iter().map(|(f, m)| f.is_some() as usize + m.is_some() as usize).sum::<usize>(); | |
| println!(" Parents: {}", parent_count); |
src/main.rs
Outdated
| lines.push("Immediate Family:".to_string()); | ||
| lines.push(format!( | ||
| " Parents: {}", | ||
| if parents.is_empty() { 0 } else { 2 } |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as in the narrow format: this assumes exactly 2 parents when any parents exist. The actual count should reflect the number of non-None parents returned by get_parents().
| if parents.is_empty() { 0 } else { 2 } | |
| parents.len() |
src/main.rs
Outdated
| println!("{msg}"); | ||
| fn print_summary(gedcom: &Gedcom, home_xref: Option<&str>, verbose: bool) { | ||
| // Get terminal width, default to 80 if unable to detect | ||
| let term_width = term_size::dimensions().map(|(w, _)| w).unwrap_or(80); |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic number 80 for default terminal width should be defined as a named constant (e.g., const DEFAULT_TERMINAL_WIDTH: usize = 80;) to improve code clarity and maintainability.
src/main.rs
Outdated
| let term_width = term_size::dimensions().map(|(w, _)| w).unwrap_or(80); | ||
|
|
||
| // Use side-by-side layout if terminal is wide enough (>= 120 columns) | ||
| if term_width >= 120 { |
Copilot
AI
Dec 12, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The threshold value 120 for wide layout should be defined as a named constant (e.g., const WIDE_LAYOUT_THRESHOLD: usize = 120;) to improve code clarity and make it easier to adjust if needed.
Feedback from PR #18 (Copilot review): 1. Remove --summary flag (confusing with --dump) - Summary is now the default behavior - Use --dump for debug output - Simplifies CLI interface 2. Fix parent count logic - Previously assumed 2 parents if any parents existed - Now correctly counts non-None parents from get_parents() - Handles cases like (Some(father), None) properly - Applied fix to both narrow and wide layout functions 3. Define terminal width constants - DEFAULT_TERMINAL_WIDTH = 80 - WIDE_LAYOUT_THRESHOLD = 120 - Improves code clarity and maintainability All tests pass, clippy clean.
Summary
This PR implements a robust command-line interface using
clapand adds a beautiful summary feature usingtabled, replacing the TODO insrc/main.rsto create pretty summary output instead of dumping the entire GEDCOM structure.Changes
New Dependencies
clap = "4.5"with derive feature - Modern CLI argument parsingtabled = "0.16"- Beautiful table formatting for statisticsCLI Flags
--version/-Vgedcom-rs 0.1.0and exits--verbose/-v--dump/-d--summary/-s--home-xref <XREF>--help/-hSummary Output Features
The new default summary output includes:
Header Information
Statistics Table
tabledValidation Warnings
--verboseis usedHome Individual Analysis
Example Output
Implementation Details
New Functions
calculate_max_generations_ancestors()- BFS traversal to find maximum ancestor depthcalculate_max_generations_descendants()- BFS traversal to find maximum descendant depthprint_summary()- Formats and displays the comprehensive summaryTechnical Approach
clap::Parserderive macro for clean, declarative CLI definitionsGedcomAPI methods:get_parents(),get_children(),get_spouses(),get_siblings()get_ancestors(),get_descendants()find_individual_by_xref()Testing
All existing tests pass. Manual testing confirms:
--versionshows version and exits--helpshows comprehensive usage information--dumpproduces debug output (old behavior)--summaryproduces formatted summary (new default)--home-xref @I3@correctly changes home individual--verboseshows detailed warningsBenefits
✅ Resolves TODO - Implements the long-standing TODO to create pretty summary
✅ Better UX - Beautiful, readable output instead of debug dump
✅ Flexible - Multiple modes (summary, dump, verbose) via flags
✅ Informative - Provides genealogy insights (generations, family counts)
✅ Professional - Clean table formatting with
tabled✅ Ergonomic - Modern CLI with
clap(help, version, etc.)✅ Backwards Compatible - Old behavior available via
--dumpBreaking Changes
--dump.