Conversation
Parse command-line arguments (--key=value) into reflectable structs. Supports nested structs, optional, vector, enum, Flatten, Rename. Automatic snake_case to kebab-case conversion via SnakeCaseToKebabCase processor.
Positional<T> marks fields as positional CLI arguments (no -- prefix). Short<_name, T> adds short aliases (e.g. -p for --port). Both wrappers are transparent for non-CLI formats (JSON, YAML, etc.) via Parser specializations that delegate to the inner type. parse_argv now categorizes args into named/short/positional buckets. resolve_args merges them into a flat map using compile-time metadata.
…ambiguity
- Fix std::forward<T> → std::move in move-assignment (Positional, Short)
- Fix move constructors copying via get() instead of moving value_
- Add static_assert: Short name must be exactly one character
- Add static_assert: disallow nested Positional<Short<...>> wrappers
- Fix split() skipping empty elements ("a,,b" → ["a", "b"])
- Add null-safety check in Reader::to_object()
- Reclaim non-boolean values from short bool flags (-v somefile)
- Add tests for bool-short-positional interaction and empty elements
- Add xml_content comment, blank line between transform functions
Summary of ChangesHello @Centimo, 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 a comprehensive command-line interface (CLI) argument parsing module to the Highlights
Changelog
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 introduces a new command-line argument parser (rfl::cli::read) that leverages reflection to parse argc/argv into reflectable structs. Key features include automatic snake_case to kebab-case conversion for field names, support for positional arguments via rfl::Positional<T>, and single-character aliases using rfl::Short<"x", T>. The implementation includes robust handling for boolean flags, comma-separated vector parsing, and a three-stage parsing architecture (parse_argv -> resolve_args -> Reader). Compile-time validation prevents nested Positional and Short wrappers, ensuring type safety. Comprehensive test cases have been added to validate the new functionality across various scenarios, including error handling, nested structures, and interactions with other rfl features like Flatten and Rename. The documentation has been updated to reflect the new CLI module's usage and capabilities.
include/rfl/Positional.hpp
Outdated
| /// The underlying type. | ||
| using Type = T; | ||
|
|
||
| Positional() : value_(Type()) {} |
There was a problem hiding this comment.
The default constructor Positional() : value_(Type()) {} does not have a requires std::is_default_constructible_v<Type> constraint. If Type is not default constructible, this will result in a compile-time error. It's good practice to explicitly state this requirement, similar to the Default constructor.
| Positional() : value_(Type()) {} | |
| Positional() requires std::is_default_constructible_v<Type> : value_(Type()) {} |
There was a problem hiding this comment.
@Centimo you can do this if you want to but I personally don't care. Feel free to just ignore Gemini on this.
include/rfl/Short.hpp
Outdated
|
|
||
| static_assert(_name.length == 1, "Short name must be exactly one character."); | ||
|
|
||
| Short() : value_(Type()) {} |
There was a problem hiding this comment.
Similar to Positional, the default constructor Short() : value_(Type()) {} lacks a requires std::is_default_constructible_v<Type> constraint. This could lead to compile errors if Type is not default constructible. Adding the constraint makes the requirements explicit.
| Short() : value_(Type()) {} | |
| Short() requires std::is_default_constructible_v<Type> : value_(Type()) {} |
|
@Centimo, This is awesome stuff! I just read through the code a bit and I love it. I'm going to give it a try later but I like what I'm seeing so far. |
I'm planning on making a few more fixes, so you probably shouldn't rush into a full-blown review. |
- Break string literal + std::to_string concatenation chain that triggers false -Warray-bounds in GCC 12 with -O3 -Werror - Accept negative numbers as short option values (-p -42) - Preserve positional argument order when reclaiming from bool short flags - Add requires constraint on Positional/Short default constructors - Add tests for negative values and multi-bool reclaim ordering
|
@Centimo , first of all, great job! Thank you so much. There are a couple of minor review comments, which should take you like 10 minutes to resolve. Very minor stuff. Also, could you update the README.md as well? This is a significant and very cool new functionality that should be highlighted in the README. |
2 participants
Summary
rfl::cli::read<T>(argc, argv)— parse command-line arguments into any reflectable struct using reflectionsnake_case→kebab-casefield name conversion (host_name→--host-name)rfl::Positional<T>wrapper for bare positional arguments matched in declaration orderrfl::Short<"x", T>wrapper for single-character aliases (-p 8080,-v)-v somefiletreatssomefileas positional, not as value of-v--tags=a,b,c), empty elements skippedparse_argv→resolve_args→ReaderPositional<Short<...>>) and multi-char short names are static_assert errors