Skip to content

THE BIG CLI REWRITE #94

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

Closed
wants to merge 2 commits into from
Closed

THE BIG CLI REWRITE #94

wants to merge 2 commits into from

Conversation

jakelang
Copy link
Collaborator

@jakelang jakelang commented May 27, 2019

WE ARE A MODERN INDEPENDENT SOFTWARE PROJECT SO LETS HAVE THE UI OF ONE

@jakelang jakelang added the enhancement New feature or request label May 27, 2019
@jakelang jakelang force-pushed the refactor-cli branch 2 times, most recently from e6622ad to 649fd3b Compare June 8, 2019 15:48
@jakelang jakelang changed the title Refactor CLI CLI Refactor Part 1 Jun 11, 2019
@jakelang jakelang changed the base branch from master to cli-refactor-staging June 11, 2019 17:23
@jakelang jakelang added the wip label Jun 20, 2019
@axic
Copy link
Member

axic commented Jul 15, 2019

@jakelang is this going to be finished?

@jakelang jakelang force-pushed the refactor-cli branch 2 times, most recently from 0a39862 to c6b65c3 Compare July 31, 2019 22:02
@jakelang jakelang changed the title CLI Refactor Part 1 THE BIG CLI REWRITE Aug 2, 2019
@jakelang jakelang added the monster Huge PR. What hath thou wrought label Aug 2, 2019
@jakelang jakelang force-pushed the refactor-cli branch 3 times, most recently from 56f3af8 to 14c7a33 Compare August 7, 2019 20:14
@axic axic changed the base branch from cli-refactor-staging to master August 29, 2019 22:23
@axic axic force-pushed the refactor-cli branch 4 times, most recently from 39d72fa to 5fc46be Compare September 8, 2019 11:56
.global(true),
)
.arg(
Arg::with_name("DEBUG_MESSAGES")
Copy link
Member

Choose a reason for hiding this comment

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

Why not keep VERBOSE as before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks strange when you run chisel --help

while let Some((name, module)) = ruleset.modules_mut().pop_front() {
chisel_debug!(1, "Executing module {}", &name);

// TODO: homogeneous module handling
Copy link
Member

Choose a reason for hiding this comment

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

I think the solution to this giant mess is passing a HashMap<String, String> as configuration as part of ChiselModule or ModulePreset.

Copy link
Member

Choose a reason for hiding this comment

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

Attempted it in #161.

@@ -93,7 +97,7 @@ impl RulesetResult {
/// Write output module to specified file if the module was mutated.
/// Returns Ok(false) if there is no mutation.
/// Returns error on writer error or invalid mode.
pub fn write(&mut self, mode: &str) -> Result<bool, Box<dyn Error>> {
pub fn write(mut self, mode: &str) -> Result<bool, Box<dyn Error>> {
Copy link
Member

Choose a reason for hiding this comment

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

Aha! :)

@axic axic force-pushed the refactor-cli branch 3 times, most recently from 32d7a44 to 43bef5f Compare September 25, 2019 19:32
fn fields(&self) -> (&String, &String) {
(&self.module_name, &self.preset)
}
fn fail(code: i32, message: &str) -> ! {
Copy link
Member

Choose a reason for hiding this comment

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

Looking at all the users of fail, all of them use code = 1. Do we need the code right now?

@jakelang
Copy link
Collaborator Author

Split into different PRs and merged.

@jakelang jakelang closed this Dec 28, 2019
@jakelang jakelang deleted the refactor-cli branch December 28, 2019 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request monster Huge PR. What hath thou wrought wip
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants