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

Allow passing filename via environment variable. #22

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

hoostus
Copy link
Contributor

@hoostus hoostus commented Aug 5, 2024

Instead of passing the filename on the command line it can be specified via the BEANCOUNT_FILE environment variable.

The motivation is: with grep having to specify the filename every time is plausible because you're going to run grep on a wide variety of files. With bean-grep (most? all?) people are only going to run bean-grep on a single file, their top-level beancount file. So having to continually specify it is a bit tedious, since in practice that argument will never change.

By allowing the filename to be passed via an environment variable we can bypass that repetition. The general idea is
you could use direnv to set BEANCOUNT_FILE when you enter your beancount directory. Or just set it globally once and for all in your shell configuration file. Then you can simply do

bean-grep -d 2024-01
bean-grep -p Lotus
bean-grep -a Vanguard -d 2024-01

Instead of passing the filename on the command
line it can be specified via the BEANCOUNT_FILE
environment variable.
@zacchiro
Copy link
Owner

zacchiro commented Aug 5, 2024

Thanks, this is a very good idea, as it is indeed tedious to have to specify the beancount file every time.

One question: why the restriction to only specify one file via the environment variable?
Is it because it would be too complex to support a file separator (and hence escaping it) in there?
I think it's fine, but I'd like to understand the rationale before integrating this.

@hoostus
Copy link
Contributor Author

hoostus commented Aug 5, 2024 via email

@zacchiro
Copy link
Owner

zacchiro commented Aug 5, 2024

Yes, I'm not sure there is a good way to handle escaping all the possible characters in filenames. But I'm open to ideas.

I mean, it's a classic escaping issue. We can pick a character as separator (e.g., a 0x20 space) and require that all spaces appearing in file names be escaped somehow (e.g., by doubling the space, or prefixing them with backslash, which would need to be escaped too). Every other weird character does not really pose a problem, it will just be up to the user how to shell-escape them properly so that they end up correctly in the environment variable.

My question was more about whether it's worth the additional complexity, and I'm leaning "no" on this.

Support for multiple files in the environment variable can also be added in the future with full backward compatibility, which is another argument for not doing this right away.

@hoostus
Copy link
Contributor Author

hoostus commented Aug 6, 2024

My question was more about whether it's worth the additional complexity, and I'm leaning "no" on this.

That was my thinking, too. Especially since it isn't clear how often someone would want to do it, given that beancount already provides an include syntax. Maybe it is worth mentioning that as a workaround? That is, you can't specify multiple files but you can set BEANCOUNT_FILE to a single file "toplevel.beancount" that simply has lines like:

include "file1.beancount"
include "file2.beancount"
include "file3.beancount"

But maybe that's enough of an edge case it isn't worth cluttering up the documentation over.

@zacchiro
Copy link
Owner

zacchiro commented Aug 6, 2024

OK, I'm sold, let's go for this. Just a couple of nitpicks before merging:

  • please use BEANCOUNT_FILENAME as variable name (rather than _FILE) as that is both more correct and more consistent with the rest of the documentation
  • in the documentation, instead of saying "The FILENAME can *also* be specified", say "The FILENAME can *alternatively* be specified", because using the envvar effectively disable the ability to bean-grep multiple files

@hoostus
Copy link
Contributor Author

hoostus commented Aug 7, 2024

I've made those changes.

@zacchiro zacchiro merged commit d31914d into zacchiro:main Aug 7, 2024
1 check passed
@zacchiro
Copy link
Owner

zacchiro commented Aug 7, 2024 via email

@hoostus
Copy link
Contributor Author

hoostus commented Aug 7, 2024 via email

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.

2 participants