Skip to content

feat/mcl/jcli #235

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

feat/mcl/jcli #235

wants to merge 12 commits into from

Conversation

monyarm
Copy link
Contributor

@monyarm monyarm commented Apr 3, 2025

  • feat(mcl): Add commandline argument support
  • feat(mcl/utils/log): Implement errorAndExit function
  • feat(mcl/utils/process): Add support to execute function, for supplying redirect; Improve execute function argument escaping, to not escape globs
  • feat(mcl/config): Implement mcl config command
  • refactor(mcl/dub): Remove deprecated dflags for improved build configuration
  • fix(ci_matrix): Initialize params directly in ci_matrix function
  • fix(utils/cachix): Modify getCachixDeploymenApiUrl definition, to allow compilation with newer d compiler versions.
  • feat(mcl/main): Integrate argparse for command-line argument parsing and enhance command handling
  • feat(commands/get_fstab): Enhance get_fstab command with argparse integration and structured argument handling
  • feat(commands/deploy_spec): Enhance deploy_spec command with argparse integration and structured argument handling
  • feat(commands/host_info): Enhance host_info command with argparse integration and structured argument handling
  • feat(commands/config): Enhance config command with argparse integration and structured argument handling
  • feat(commands/machine): Enhance machine command with argparse integration and structured argument handling
  • feat(commands): Enhance ci/matrix commands with argparse integration and structured argument handling where applicable

@monyarm monyarm force-pushed the feat/mcl/jcli branch 2 times, most recently from 662b512 to a999476 Compare April 10, 2025 14:55
@PetarKirov PetarKirov mentioned this pull request Jun 4, 2025
Copy link
Contributor

github-actions bot commented Jun 4, 2025

Thanks for your Pull Request!

This comment will be updated automatically with the status of each package.

@reo101 reo101 self-requested a review June 4, 2025 15:57
Comment on lines +29 to +42
const char[] genSubCommandArgs =
"@SubCommands\n"~
"SumType!("~
"get_fstab_args,"~
"deploy_spec_args,"~
"host_info_args,"~
"config_args,"~
"machine_args,"~
"ci_matrix_args,"~
"ci_args,"~
"print_table_args,"~
"shard_matrix_args,"~
"Default!unknown_command_args"~
") cmd;";
Copy link
Member

Choose a reason for hiding this comment

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

Could this be done in a similar fashion to the following genSubCommandMatch template?

LogLevel logLevel = LogLevel.info;
args.getopt("log-level", &logLevel);
@(Command(" ").Description(" "))
struct unknown_command_args {}
Copy link
Member

Choose a reason for hiding this comment

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

Prefer capitalized names (PascalCase, to be exact) for structs. Applies to the rest of the _args structs too.

Comment on lines -189 to +195
export void print_table(string[] args)
@(Command("print-table", "print_table").Description("Print a table of the cache status of each package"))
struct print_table_args
{
}

export int print_table(print_table_args args)
Copy link
Member

Choose a reason for hiding this comment

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

Can we instead try supporting argument-less commands by providing an aptly argument-less function returning void, with genSubCommandArgs being updated to dynamically check whether or not each sub-command handler function has arguments/return value at all before taking them into account?
This also applies to all _args structs annotated with @(Command(" ").Description(" ")).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants