Fix build failures, config loading, and wire remaining CLI commands#17
Fix build failures, config loading, and wire remaining CLI commands#17bashandbone merged 5 commits intocli-improvementsfrom
Conversation
…ommands Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes multiple compilation errors and a broken configuration round-trip in the submod CLI tool, and wires up previously stubbed CLI commands (GenerateConfig and NukeItFromOrbit).
The root cause of most integration test failures was a SubmoduleEntries serialize/deserialize mismatch and incorrect use of Figment's .nested() API. Both gix and git2 add_submodule implementations are changed to intentionally return Err to force the existing CLI fallback path.
Changes:
src/config.rs: Added customSerializeimpl forSubmoduleEntries(flat name→entry map), removed.nested()from Figment config loading, and added missingserdeimportssrc/commands.rs: Added requirednamefields toDelete/Disablevariants, removed invalidvalue_hintonReset.all, changedfrom_setupfromStringtoOption<String>src/main.rs/src/utilities.rs/src/options.rs/src/git_ops/*.rs: Fixed compilation errors (duplicate bindings, missing imports, lifetime issue,trimmedbinding), implementedGenerateConfigandNukeItFromOrbithandlers, and made gix/git2add_submodulefall through to the CLI
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/config.rs |
Custom SubmoduleEntries::Serialize, .nested() removal, added missing use items |
src/commands.rs |
Added name to Delete/Disable, fixed Reset.all, changed from_setup to Option<String> |
src/main.rs |
Removed duplicate bindings and unused imports, added clap_complete::generate, implemented GenerateConfig/NukeItFromOrbit handlers |
src/utilities.rs |
Refactored get_current_branch with inner helper to fix lifetime error; replaced /** doc-comment blocks |
src/options.rs |
Added let trimmed = options.trim() before use in from_gitmodules |
src/git_ops/gix_ops.rs |
add_submodule now returns Err to force CLI fallback |
src/git_ops/git2_ops.rs |
add_submodule now returns Err to force CLI fallback |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // gix does not support cloning in add_submodule; fall through to git2/CLI. | ||
| Err(anyhow::anyhow!( | ||
| "gix add_submodule not implemented: use git2 or CLI fallback for '{}'", | ||
| opts.name | ||
| )) |
There was a problem hiding this comment.
The error message "gix add_submodule not implemented: use git2 or CLI fallback for '...'" is an internal implementation detail that will be printed to stderr via the eprintln! in try_with_fallback_mut every time a user runs add. A more user-friendly approach would be to use a sentinel error type (e.g., a specific variant or a recognized message like "not implemented") that try_with_fallback_mut can detect and suppress the stderr log for, since this fallback is always expected — it is not a real failure. Similarly for the git2 error message.
| use std::str::FromStr; | ||
| use submod::options::SerializableBranch; | ||
|
|
||
| use clap_complete::generate; |
There was a problem hiding this comment.
A blank line is missing between the use clap_complete::generate; import and the fn main() definition. In Rust convention and throughout the rest of this file, import blocks are separated from function definitions by a blank line. This is a minor formatting issue but inconsistent with the surrounding code style.
| use clap_complete::generate; | |
| use clap_complete::generate; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The codebase had multiple compilation errors and a broken config round-trip that caused all submodule lookups to silently return empty. Most integration tests were failing as a result.
Config loading (root cause of most failures)
SubmoduleEntriesserialize/deserialize mismatch:#[derive(Serialize)]emitted struct field names (submodules,sparse_checkouts) while the customDeserializetreated all map keys as submodule names. Figment's default-merging round-tripped these field names back as phantom submodule entries. Added a customSerializeimpl that emits a flatname → entrymap..nested()in Figment:Toml::file(path).nested()wrapped TOML section names as Figment profile keys, making every[my-lib]section invisible on extraction. Removed.nested().Add submodule not cloning
Both
gixandgit2add_submoduleimplementations returnedOk(())without cloning:gix: only wrote.gitmodules, no clonegit2: left partial filesystem state (.gitfile written, no fetch), causing the CLI fallback to fail on re-entryBoth now return
Err(...)to fall through to thegit submodule addCLI fallback, which is reliable.Compilation errors
config.rs: missinguse serde::de::Deserializer/use serde::ser::SerializeMapoptions.rs:trimmedused before binding infrom_gitmodules()— addedlet trimmed = options.trim()utilities.rs: lifetime error from borrowing a temporary inget_current_branch(); refactored with an inner helper to eliminate the issue and the duplicated head-reading logicmain.rs: duplicateletbindings, missinguse clap_complete::generate,boolpassed whereOption<bool>expectedCLI command fixes
Delete/Disablevariants were missing their requirednamefieldReset.all: invalidvalue_hinton a boolean flag (clap panics at runtime)GenerateConfig.from_setup: was required; madeOption<String>to support empty-config generationGenerateConfigandNukeItFromOrbitcommand handlers (both previously returned a "not yet implemented" error)/**doc-comment blocks inutilities.rsthat Rust was compiling as doctests🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.