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

Enable click's auto_envvar_prefix #20

Closed
wants to merge 2 commits into from
Closed

Conversation

hoostus
Copy link
Contributor

@hoostus hoostus commented Aug 2, 2024

Enable click's auto_envvar_prefix which allows bean-grep's command line options to also be set via environment variables.

The prefix is BEANGREP so the environment variables are BEANGREP_VERBOSE, BEANGREP_CASE_SENSITIVE, etc.

This fixes #8

n.b. I just realised I haven't updated the documentation for this but if the PR is accepted I can do that.

The environment variables could either be set permanently & globally via your shell config file or you could use something direnv to set them just while inside your beancount directory.

I think the current defaults actually work fine and don't see a need for this. However, this is scaffolding for my next pull request which will let you also specify the beancount file via an environment.

hoostus added 2 commits August 2, 2024 16:26
bean-grep's command line options to also be set
via environment variables.

This fixes zacchiro#8
This allows the beancount file to be specified
via an environment variable BEANCOUNT_FILE, so
you don't need to repeat it every time you run
bean-grep.

As part of this change, the PATTERN is now
mandatory. This is because I don't
really understand how to make click keep working
the old way while also allowing the filename
to come via an environment variable. We could
pull the environment variable in ourselves manually
but it feels cleaner to let click do that.

I'm not sure if there's really a usecase for not
having a pattern, anyway? That's just cat. I don't
think regular grep allows no pattern?
@hoostus
Copy link
Contributor Author

hoostus commented Aug 2, 2024

Hm, I'm clearly not very good at github since I ended up including both changes in this single PR. Let me know if you want me to rework anything......

@zacchiro
Copy link
Owner

zacchiro commented Aug 2, 2024

Thanks a lot for this, which would indeed be a good (easy) way to address defaults.
Please make the PR clean indeed, so that it only include one commit. (It is also fine if you don't want to use GitHub: just send me the output of git format-patch and I will be happy to apply it locally and push.)

More importantly though, I'd like to have at least a couple of tests for this, as it's something that can easily break unnoticed in the future. Can you give it a try? (I'm assuming click provides a way to set envvar via its CLI runner, but I never looked into it.)

@hoostus hoostus closed this Aug 3, 2024
@hoostus
Copy link
Contributor Author

hoostus commented Aug 3, 2024

Give me a day or two to rework this (busy with family over the weekend so probably not until next week).

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.

Support configuration file, to override defaults
2 participants