Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

refactor: use bpaf #4405

Merged
merged 17 commits into from
Apr 28, 2023
Merged

refactor: use bpaf #4405

merged 17 commits into from
Apr 28, 2023

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Apr 24, 2023

Summary

This PR refactors our CLI arguments parsing, using bpaf. pico_args doesn't have all the features we need, and it's very difficult to maintain help commands, documentation, all the arguments we want, etc.

Here's a list of the reactors I have done:

  • use the derive feature of bpaf;
  • the whole Configuration struct is now tagged with all the bpaf directives we need. All the properties that don't need to be exposed as CLI arguments have #[bpaf(hide)]. Although hiding them is not enough, all the chains of types inside a struct must be handled via bpaf directives, which means the lint rules must also be hidden. On the other hand, this could allow us to expose them via CLi if we want to;
  • The description/documentation of commands and arguments are all provided via Rust docs, so I copied what we had in the help.rs file in the new Configuration struct;
  • Created a new CliOptions which will contain only options related to the CLI, like colors, max-diagnostics , etc.
  • All the commands have been declared inside a new type called Command;

Test Plan

Most current tests must stay the same to avoid regressions. The only snapshot tests I have updated are those related to the arguments that fail the parsing. I updated most of the code of tests to adapt them to use bpaf.

I added new tests for each command's --help output. This will help us to understand which arguments we want to show.

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Apr 24, 2023

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit aca6803
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/644b77f48ba18800082cc98d

@github-actions github-actions bot added A-CLI Area: CLI A-Diagnostic Area: errors and diagnostics A-Project Area: project configuration and loading A-Tooling Area: our own build, development, and release tooling labels Apr 24, 2023
@github-actions
Copy link

github-actions bot commented Apr 24, 2023

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48863 48863 0
Passed 47796 47796 0
Failed 1067 1067 0
Panics 0 0 0
Coverage 97.82% 97.82% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6212 6212 0
Passed 1763 1763 0
Failed 4449 4449 0
Panics 0 0 0
Coverage 28.38% 28.38% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 573 573 0
Failed 66 66 0
Panics 0 0 0
Coverage 89.67% 89.67% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17224 17224 0
Passed 13122 13122 0
Failed 4102 4102 0
Panics 0 0 0
Coverage 76.18% 76.18% 0.00%

@github-actions github-actions bot added the A-Core Area: core label Apr 27, 2023
@Conaclos Conaclos mentioned this pull request Apr 27, 2023
1 task
@ematipico ematipico marked this pull request as ready for review April 27, 2023 21:20
@ematipico ematipico merged commit 4bae895 into main Apr 28, 2023
@ematipico ematipico deleted the feat/bpaf-cli branch April 28, 2023 07:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-CLI Area: CLI A-Core Area: core A-Diagnostic Area: errors and diagnostics A-Project Area: project configuration and loading A-Tooling Area: our own build, development, and release tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant