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 root env vars to fix --color=always problem #263

Merged
merged 1 commit into from
Nov 16, 2024

Conversation

joshka
Copy link
Contributor

@joshka joshka commented Nov 16, 2024

Adds a new root level config item env, which allows setting environment
vars for all jobs. This makes it possible then to set CARGO_TERM_COLOR
to always, which fixes the problem with --color=always not working.

Fixes: #261 and #124

Adds a new root level config item env, which allows setting environment
vars for all jobs. This makes it possible then to set CARGO_TERM_COLOR
to always, which fixes the problem with --color=always not working.

Fixes: Canop#261 and Canop#124
@Canop
Copy link
Owner

Canop commented Nov 16, 2024

I'll test a little more today, but this looks very good.

@Canop
Copy link
Owner

Canop commented Nov 16, 2024

Something which may seem a little disturbing: if you remove the env.CARGO_TERM_COLOR = "always" line from the bacon.toml of your project, the variable is still set, because the default bacon.toml is used to init settings and env vars are only extended.

This doesn't look like it can be a problem, it's even good as it will make it possible for users to add new jobs without the --color always thing even if they don't have the env.color = always line at the top.

But the fact that you can't, today, remove an env var defined previously (in the prefs.toml or in the bacon.toml root) may be seen as a problem. Maybe we'll have to add support for an ugly inherit_env = false in the future ?

Right now, I think what you did is the way to go.

@Canop Canop merged commit 35d1866 into Canop:main Nov 16, 2024
@joshka
Copy link
Contributor Author

joshka commented Nov 16, 2024

Yeah, I was thinking about that as a potential problem too. Perhaps making the hashmap accept Option<String> would work for unsetting vars.

I also didn't add env to the args, which might be nice too, mainly because I wasn't sure the syntax for representing --env FOO=bar in clap (and didn't find an obvious answer in a few minutes of search)

@joshka joshka deleted the jm/color-always branch November 19, 2024 00:57
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.

--color always revisited
2 participants