feat!(spec): add support for license, before/after help metadata#542
feat!(spec): add support for license, before/after help metadata#542
Conversation
Add parsing, serialization, and merge support for top-level metadata fields: license, before_help, after_help, before_long_help, after_long_help. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces comprehensive support for several previously documented but unimplemented top-level metadata fields within the specification parsing and serialization logic. By integrating Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for several new top-level metadata fields: license, before_help, after_help, before_long_help, and after_long_help. The changes correctly handle parsing, serialization, and merging for these new fields.
My review includes two main points:
- A suggestion to add markdown variants (e.g.,
before_help_md) for the new help-related fields to maintain consistency with existing patterns in the codebase. - A refactoring suggestion to reduce code duplication in the serialization logic by using a macro.
Overall, the changes are a good addition and address the missing functionality.
lib/src/spec/mod.rs
Outdated
| pub license: Option<String>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub before_help: Option<String>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub after_help: Option<String>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub before_long_help: Option<String>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub after_long_help: Option<String>, |
There was a problem hiding this comment.
For consistency with about_md and the fields in SpecCommand, consider adding markdown variants for the new help-related fields (e.g., before_help_md, after_help_md, before_long_help_md, after_long_help_md). This would allow for markdown-formatted help text at the top level, which seems to be a pattern in this crate. If you add these, remember to update the parsing, merging, and serialization logic accordingly.
| if let Some(license) = &self.license { | ||
| let mut node = KdlNode::new("license"); | ||
| node.push(KdlEntry::new(KdlValue::String(license.clone()))); | ||
| nodes.push(node); | ||
| } | ||
| if let Some(before_help) = &self.before_help { | ||
| let mut node = KdlNode::new("before_help"); | ||
| node.push(KdlEntry::new(KdlValue::String(before_help.clone()))); | ||
| nodes.push(node); | ||
| } | ||
| if let Some(after_help) = &self.after_help { | ||
| let mut node = KdlNode::new("after_help"); | ||
| node.push(KdlEntry::new(KdlValue::String(after_help.clone()))); | ||
| nodes.push(node); | ||
| } | ||
| if let Some(before_long_help) = &self.before_long_help { | ||
| let mut node = KdlNode::new("before_long_help"); | ||
| node.push(KdlEntry::new(KdlValue::String(before_long_help.clone()))); | ||
| nodes.push(node); | ||
| } | ||
| if let Some(after_long_help) = &self.after_long_help { | ||
| let mut node = KdlNode::new("after_long_help"); | ||
| node.push(KdlEntry::new(KdlValue::String(after_long_help.clone()))); | ||
| nodes.push(node); | ||
| } |
There was a problem hiding this comment.
This block of code is highly repetitive. To improve maintainability and reduce code duplication, you can use a local macro to generate these KdlNodes for optional fields. This will make the code cleaner and easier to modify, especially if you add the _md variants as suggested in another comment.
macro_rules! push_opt_node {
($nodes:expr, $field:expr, $name:expr) => {
if let Some(value) = &$field {
let mut node = KdlNode::new($name);
node.push(KdlEntry::new(value.clone()));
$nodes.push(node);
}
};
}
push_opt_node!(nodes, self.license, "license");
push_opt_node!(nodes, self.before_help, "before_help");
push_opt_node!(nodes, self.after_help, "after_help");
push_opt_node!(nodes, self.before_long_help, "before_long_help");
push_opt_node!(nodes, self.after_long_help, "after_long_help");
Greptile SummaryThis PR adds parsing, merge, serialization, and docs-model propagation for five new top-level spec metadata fields ( Key observations:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[KDL Input] --> B{Parse node key}
B -->|license| C[schema.license]
B -->|before_help| D[schema.before_help]
B -->|after_help| E[schema.after_help]
B -->|before_long_help\nbefore_help_long| F[schema.before_help_long]
B -->|after_long_help\nafter_help_long| G[schema.after_help_long]
C & D & E & F & G --> H[crate::Spec]
H -->|Spec::merge| H
H -->|Display / KDL serialize| I[KDL Output\nnormalized keys]
H -->|From impl| J[docs::models::Spec]
J --> K[Tera Template\nraw text only]
J --> L[Spec::render_md\nonly about_md rendered]
style L fill:#ffe0b2,stroke:#e65100
style I fill:#e8f5e9,stroke:#2e7d32
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds support in the Rust spec parser/serializer for several top-level metadata keys that are documented in the spec reference but previously rejected as unsupported, addressing issue #537.
Changes:
- Extend
Specwith optional top-level metadata fields:license,before_help,after_help,before_long_help,after_long_help - Teach the KDL parser to recognize these keys and populate the
Spec - Include these fields in
Spec::mergeandDisplay(KDL serialization)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/src/spec/mod.rs
Outdated
| "license" => schema.license = Some(node.arg(0)?.ensure_string()?), | ||
| "before_help" => schema.before_help = Some(node.arg(0)?.ensure_string()?), | ||
| "after_help" => schema.after_help = Some(node.arg(0)?.ensure_string()?), | ||
| "before_long_help" => schema.before_long_help = Some(node.arg(0)?.ensure_string()?), | ||
| "after_long_help" => schema.after_long_help = Some(node.arg(0)?.ensure_string()?), |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #542 +/- ##
==========================================
- Coverage 77.94% 72.37% -5.57%
==========================================
Files 48 48
Lines 6682 6831 +149
Branches 6682 6831 +149
==========================================
- Hits 5208 4944 -264
- Misses 1114 1243 +129
- Partials 360 644 +284 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Rename before_long_help/after_long_help to before_help_long/after_help_long to match SpecCommand convention - Accept both KDL spellings (before_long_help and before_help_long) - Add all new fields to docs::models::Spec and its From<crate::Spec> impl Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…aking semver Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| use crate::{SpecArg, SpecComplete, SpecFlag}; | ||
|
|
||
| #[derive(Debug, Default, Clone, Serialize)] | ||
| #[non_exhaustive] |
There was a problem hiding this comment.
#[non_exhaustive] is a semver-breaking change
Adding #[non_exhaustive] to a public struct with all-public fields is a backward-incompatible change. Any external crate that constructs a Spec instance using struct literal syntax—including the struct update form Spec { before_help: Some("...".into()), ..Default::default() }—will fail to compile after this change.
The other #[non_exhaustive] usage in this codebase (Parser in parse.rs) only has private fields, so the attribute was effectively a no-op there. Here it has real impact because all Spec fields are pub.
If this is intentional (e.g., to future-proof the API for adding fields without semver bumps going forward), it still requires a semver-major bump per the SemVer compatibility rules for Rust crates, or at minimum a CHANGELOG/migration note. If unintentional, the attribute should be removed.
jdx
changed the title
### 🚀 Features - **(spec)** **breaking** add support for license, before/after help metadata by [@jdx](https://github.com/jdx) in [#542](#542) ### 🐛 Bug Fixes - **(cobra)** escape newlines, tabs, and carriage returns in kdlQuoteAlways by [@thecodesmith](https://github.com/thecodesmith) in [#539](#539) - bump major version for breaking changes in release automation by [@jdx](https://github.com/jdx) in [#544](#544) - add custom_major_increment_regex for breaking change detection by [@jdx](https://github.com/jdx) in [#545](#545) - handle all breaking change commit formats in major bump regex by [@jdx](https://github.com/jdx) in [27e1ab1](27e1ab1) - normalize breaking change commit format in preprocessor by [@jdx](https://github.com/jdx) in [aa72b92](aa72b92) ### 📚 Documentation - add argparse-usage integration by [@jdx](https://github.com/jdx) in [#531](#531) - mark KDL code blocks as KDL and use correct inline-comment `//` by [@muzimuzhi](https://github.com/muzimuzhi) in [#536](#536) - fix include syntax to match implementation by [@jdx](https://github.com/jdx) in [#540](#540) - consolidate integration list to single source by [@jdx](https://github.com/jdx) in [#541](#541) - fix link to integrations by [@muzimuzhi](https://github.com/muzimuzhi) in [#543](#543) ### 🛡️ Security - **(deps)** update dependency eslint to v10 by [@renovate[bot]](https://github.com/renovate[bot]) in [#526](#526) ### 🔍 Other Changes - Added an integration with ruby's OptionParser by [@packrat386](https://github.com/packrat386) in [#533](#533) ### 📦️ Dependency Updates - update actions/setup-node digest to 53b8394 by [@renovate[bot]](https://github.com/renovate[bot]) in [#525](#525) - update jdx/mise-action action to v3 by [@renovate[bot]](https://github.com/renovate[bot]) in [#528](#528) - update rust crate roff to v1 by [@renovate[bot]](https://github.com/renovate[bot]) in [#529](#529) ### New Contributors - @thecodesmith made their first contribution in [#539](#539) - @packrat386 made their first contribution in [#533](#533)
Summary
license,before_help,after_help,before_long_help,after_long_helpCloses #537
Test plan
cargo build --allpassescargo test --all --all-featurespassescargo clippy --all --all-features -- -D warningspassesusage lintno longer errors on specs using these fields🤖 Generated with Claude Code
Note
Low Risk
Low risk, additive metadata fields that extend spec parsing/serialization and merge logic without changing existing required behavior. Main risk is minor formatting/roundtrip differences when emitting KDL for specs that use the new fields.
Overview
Adds support for new top-level spec metadata:
license,before_help,after_help,before_help_long/before_long_help, andafter_help_long/after_long_help.Updates the spec parser to recognize these keys, includes them in
Spec::merge, and round-trips them inDisplay(KDL output). Docs models are extended so generated docs/JSON include the new fields, andSpecis marked#[non_exhaustive]for forward-compatible API evolution.Written by Cursor Bugbot for commit b5ead4a. This will update automatically on new commits. Configure here.