Skip to content

Introduce CliArgs (new design) and use to further generalize ThreeBusBasicJson example#341

Draft
PhilipFackler wants to merge 2 commits intodevelopfrom
PhilipFackler/threebusbasic-cliargs
Draft

Introduce CliArgs (new design) and use to further generalize ThreeBusBasicJson example#341
PhilipFackler wants to merge 2 commits intodevelopfrom
PhilipFackler/threebusbasic-cliargs

Conversation

@PhilipFackler
Copy link
Collaborator

@PhilipFackler PhilipFackler commented Feb 23, 2026

Description

This is another step toward a generalized application to replace examples.

Proposed changes

Use a new class CliArgs to easily define command line options.

Checklist

  • All tests pass.
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows GridKit™ style guidelines.
  • There are unit tests for the new code.
  • The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

Note that this is a draft PR. I wanted to get feedback on the design of CliArgs, and (if we want to keep it) I need to add documentation and testing for it. I would also add the capability to handle positional arguments.

@PhilipFackler PhilipFackler added enhancement New feature or request examples labels Feb 23, 2026
@pelesh pelesh self-requested a review February 23, 2026 22:18
Copy link
Collaborator

@pelesh pelesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very welcome and long overdue update. Thanks for taking initiative on this. A few comments:

  • Please document new functions with more detailed comments in the code.
  • Consider separate source/header files for different classes. Some of the classes are only a few rows but it might be a good idea to stick to "one class per file" rule.
  • I made some style suggestions that clang-format will not fix.

Comment on lines +16 to +19
class ArgValue
{
public:
ArgValue() = default;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we put classes ArgValue and ArgVector in separate header and source files.

@PhilipFackler PhilipFackler force-pushed the PhilipFackler/threebusbasic-cliargs branch from 216b243 to 7e50d41 Compare February 25, 2026 16:53
@PhilipFackler PhilipFackler force-pushed the PhilipFackler/threebusbasic-cliargs branch from 7e50d41 to 326c561 Compare February 25, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request examples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants