Skip to content
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

add --config-dir support to shellclear cli #40

Merged
merged 4 commits into from Aug 24, 2022
Merged

add --config-dir support to shellclear cli #40

merged 4 commits into from Aug 24, 2022

Conversation

boaz-quotient
Copy link
Contributor

@boaz-quotient boaz-quotient commented Aug 22, 2022

I am not sure if this exists or is unnecessary but I couldn't figure out how to set the configuration files directory through the CLI and I want to save it in a different location than homedir.
So I decided to add this to the code, let me know if that works out for you.
Thanks.

Signed-off-by: bshuster <bshuster@quotient.com>
@@ -28,6 +28,17 @@ impl Default for Config {
}
}

impl From<Option<&str>> for Config {
fn from(config_dir_path_option: Option<&str>) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

Naming: you can just use config_dir as the parameter name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jondot thanks for your review.
I updated the variable name and fixed the linting issue by running cargo fmt as you suggested.
I wonder if I can auto-format rust code in vscode (I installed rust-analyzer plugin).

Copy link
Member

Choose a reason for hiding this comment

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

Yes, add the following

Format all languages on save:

  "editor.formatOnSave": true,

For rust, use the rust-analyzer formatter:

 "[rust]": {
    "editor.defaultFormatter": "rust-lang.rust-analyzer"
  },

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also briefly updated the README file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, rust code is now auto-formatted on vscode 😆

@jondot
Copy link
Member

jondot commented Aug 22, 2022

@boaz-quotient I've approved running CI, you can take a look at the linting results for example, which might hint your editor isn't using fmt automatically. Would you like help with that?

@codecov-commenter
Copy link

Codecov Report

Merging #40 (246eb71) into main (6d6c7f1) will decrease coverage by 0.50%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main      #40      +/-   ##
==========================================
- Coverage   65.74%   65.24%   -0.51%     
==========================================
  Files          17       17              
  Lines        1559     1571      +12     
==========================================
  Hits         1025     1025              
- Misses        534      546      +12     
Impacted Files Coverage Δ
shellclear/src/bin/cmd/default.rs 0.00% <0.00%> (ø)
shellclear/src/bin/shellclear.rs 1.12% <0.00%> (ø)
shellclear/src/config.rs 88.51% <0.00%> (-3.10%) ⬇️
shellclear/src/dialog.rs 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: bshuster <bshuster@quotient.com>
@boaz-quotient
Copy link
Contributor Author

Sorry for these chaotic updates. I changed the unit tests to use snapshot + added one more unit test (hopefully this will make the coverage happy)

@jondot
Copy link
Member

jondot commented Aug 22, 2022

No worries, everything looks good to me 👍
@kaplanelad is leading this project, when he'll be back from vacation he'll probably cut a new release :)

@boaz-quotient
Copy link
Contributor Author

@jondot "ein bha-ya". No worries it's alright. I personally enjoyed practicing Rust on this project. Thanks for the opportunity.

@kaplanelad kaplanelad merged commit 09a961e into rusty-ferris-club:main Aug 24, 2022
@kaplanelad
Copy link
Member

Thanks, @boaz-quotientm bumped a new version 0.4.1/

Thanks @jondot for the backup :)

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.

None yet

4 participants