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

Env var should take precedence over config #1152

Closed
bram2000 opened this issue Aug 26, 2020 · 6 comments · Fixed by #2381
Closed

Env var should take precedence over config #1152

bram2000 opened this issue Aug 26, 2020 · 6 comments · Fixed by #2381
Labels
feature-request New feature or request help wanted Extra attention is needed

Comments

@bram2000
Copy link

Hello,

i would like to set a default theme in my bat config file, but override it occasionally with the BAT_THEME env var. Currently it seems that if the theme is set in the config then env is ignored. This seems wrong to me, i think env should take precedence over the config file (as would usually be the case to allow customisation per shell instance).

Thanks,
Bram

@bram2000 bram2000 added the feature-request New feature or request label Aug 26, 2020
@sharkdp
Copy link
Owner

sharkdp commented Sep 7, 2020

Thank you for reporting this.

i think env should take precedence over the config file (as would usually be the case to allow customisation per shell instance).

Hm. Is this really the most common behavior of other command line programs? It does sound reasonable, but for some reason I thought it should be "environment variables < config file < command line arguments". But you might be right.

@mouse07410
Copy link

He's right: the correct priority is "config files < env vars < command line flags"

@sharkdp
Copy link
Owner

sharkdp commented Sep 20, 2020

I believe you, but it would be great to see some actual examples.

@sharkdp sharkdp added the help wanted Extra attention is needed label Sep 20, 2020
@bram2000
Copy link
Author

bram2000 commented Sep 28, 2020

git is a nice example...

e.g. the man page for git-config states:

The user.name and user.email variables determine what ends up in the
           author and committer field of commit objects. If you need the author or
           committer to be different, the author.name, author.email, committer.name
           or committer.email variables can be set. Also, all of these can be
           overridden by the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, GIT_COMMITTER_NAME,
           GIT_COMMITTER_EMAIL and EMAIL environment variables.

The environment should always take priority over config files. Git is full of other examples of this.

It makes sense if you think about it; config files are like global settings, env vars apply settings to the entire shell session, and command line args are per command settings.

@sharkdp
Copy link
Owner

sharkdp commented Oct 1, 2020

Ok, thank you. In this case, let's make the necessary changes (and add some tests!). I'm afraid the change might not be completely trivial though.

jacobmischka added a commit to jacobmischka/bat that referenced this issue Oct 6, 2020
Parses env configurations into matches (for those that can be configured
via matches) instead of relying on dedicated env checks everywhere.

Fixes sharkdp#1152
jacobmischka added a commit to jacobmischka/bat that referenced this issue Oct 6, 2020
Parses env configurations into matches (for those that can be configured
via matches) instead of relying on dedicated env checks everywhere.

Fixes sharkdp#1152
jacobmischka added a commit to jacobmischka/bat that referenced this issue Oct 7, 2020
Parses env configurations into matches (for those that can be configured
via matches) instead of relying on dedicated env checks everywhere.

Fixes sharkdp#1152
jacobmischka added a commit to jacobmischka/bat that referenced this issue Jan 2, 2022
Parses env configurations into matches (for those that can be configured
via matches) instead of relying on dedicated env checks everywhere.

Fixes sharkdp#1152
@Enselic
Copy link
Collaborator

Enselic commented Aug 28, 2022

If anyone wants to tackle this, make sure to also take a look at #1281. I am closing that PR now to make the PR inbox cleaner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request help wanted Extra attention is needed
Projects
None yet
4 participants