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 RUSTUP_TERM_COLOR as an override environment variable #3435

Merged
merged 3 commits into from
Aug 17, 2023

Conversation

rami3l
Copy link
Member

@rami3l rami3l commented Aug 13, 2023

Closes #3433, getting my feet wet at the same time.

The variable name and the description are directly copied from the Cargo Book.

Concerns

  • Is the naming of the variable ideal?
  • Are there any test cases to be added for this change?
  • Should we add a CLI flag (e.g. --color) for this as well?
    • If so, should we use Clap's env feature?

@rbtcollins
Copy link
Contributor

Thank you for the patch.

Is the naming of the variable ideal?

I think its fine.

Is there any test cases to be added for this change?

yes, unit tests in terminalsource would be best I think.

Should we add a CLI flag (e.g. --color) for this as well?

We generally don't expose variables as CLI parameters.

@rami3l rami3l marked this pull request as ready for review August 15, 2023 02:00
@rami3l
Copy link
Member Author

rami3l commented Aug 15, 2023

Hmmm. IMO that CI error has nothing to do with the changes I've made... (cc #3437)

@rami3l rami3l requested a review from rbtcollins August 15, 2023 04:53
@weihanglo
Copy link
Member

(drive-by comment)

Some related issue in rust-lang/cargo, if we want to make sure the behavior is in sync.

@rbtcollins rbtcollins merged commit 94d98a4 into rust-lang:master Aug 17, 2023
16 checks passed
@rami3l rami3l deleted the feat/force-color-output branch August 17, 2023 23:18
@rami3l rami3l mentioned this pull request Jan 29, 2024
1 task
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.

Launching rustup from Command does not return a colored output
3 participants