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

Support automatic config loading via toml file #48

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

Conversation

iamriel
Copy link

@iamriel iamriel commented Apr 2, 2023

These are the main changes covered in this Pull Request:

  • Added a feature to automatically read configuration from a toml file
  • Broken down main.rs to several smaller modules
  • Added documentations

@iamriel
Copy link
Author

iamriel commented Apr 2, 2023

Hello, thank you for this package as it's improved my workflow a lot. I have forked the repository and made some changes (not sure if it's an improvement though)

I am not an expert in rust so I won't lie and I did use the help of chatgpt in some of the code changes. I am not expecting that my changes will go to your repository but I just want someone to check the implementation I made and of course accept feedback and suggestions for me to learn more.

Also, I haven't read some contributing guides in the code so forgive me if this method is not appropriate.

Thanks a lot!

@iamriel iamriel marked this pull request as draft April 2, 2023 08:37
@disrupted
Copy link
Owner

Hi @iamriel,
thanks for your contribution!

I am happy to give this a thorough look, hopefully very soon. Could you meanwhile take a look at the CI run and fix the clippy diagnostics?

@iamriel
Copy link
Author

iamriel commented Apr 12, 2023

@disrupted Absolutely, I forgot to install pre commit hooks, sorry about that. I have pushed an update in this commit. Thanks!

@iamriel iamriel marked this pull request as ready for review April 12, 2023 06:06
@mrparalon
Copy link

Really like this feature!
Any news when it could be merged?

src/config.rs Outdated
let mut path = env::current_dir().unwrap();
let target_file_name = target_file_name.unwrap_or("pyproject.toml");

while path.pop() {

Choose a reason for hiding this comment

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

This will pop current dir before check.

For example, I have config in my current dir /project/name/pyproject.toml

In this case first check will be on /project/pyproject.toml

This works ok

        loop {
            let potential_config_path = path.join(target_file_name);
            if potential_config_path.is_file() {
                return Config::from_file(potential_config_path)
                    .expect("Failed to load config file");
            }
            match path.pop() {
                true => (),
                false => return Config { tool: None },
            }
        }

Copy link
Author

Choose a reason for hiding this comment

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

@mrparalon Thanks for the feedback. I have pushed an update in this commit.

Repository owner deleted a comment from Obasoro Feb 20, 2024
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.

3 participants