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.

Sign up for GitHub

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

Jump to bottom

🌈 cargo: make command output with color #481

Closed
wants to merge 1 commit into from
Closed

🌈 cargo: make command output with color #481

wants to merge 1 commit into from

Conversation

evaporei
Copy link

@evaporei evaporei commented Jan 3, 2019 •
edited
Loading

All cargo calls were without color because
they were missing the --color flag with the
always value.

Examples of the terminal with color 🌈

  • Success
    screenshot from 2019-01-03 19-58-12

  • Error
    screenshot from 2019-01-03 20-57-06

Make sure these boxes are checked! 📦✅

  • You have the latest version of rustfmt installed
$ rustup component add rustfmt-preview --toolchain nightly
  • You ran cargo fmt on the code base before submitting
  • You reference which issue is being closed in the PR text

✨✨ 😄 Thanks so much for contributing to wasm-pack! 😄 ✨✨

This pull request closes this issue: #480

cargo: make command output with color
ff01752 
All `cargo` calls were without color because
they were missing the `--color` flag with the
`always` value.
@ashleygwilliams ashleygwilliams added this to the 0.6.0 milestone Jan 3, 2019
ashleygwilliams
ashleygwilliams reviewed Jan 3, 2019
View reviewed changes
Copy link
Member

@ashleygwilliams ashleygwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me- @alexcrichton any thoughts? not sure if there's console support things to consider with this as i'm not sure how this flag works under the hood.

@alexcrichton
Copy link
Contributor

alexcrichton commented Jan 4, 2019 •
edited
Loading

Unfortunately this is very caught up in rust-lang/rust#57337 #298 and doesn't actually fix the problem on Windows. I would personally not recommend merging this PR because it only fixes unix platforms (and even there, not completely because there's no way to turn colors off), and it doesn't add colors for Windows.

@evaporei
Copy link
Author

evaporei commented Jan 4, 2019

@alexcrichton , I can look how to fix the issue on Windows too, I think I can do it 🙂

@alexcrichton
Copy link
Contributor

Er sorry I had the wrong link there, the right issue this is caught up in is #298. @otaviopace this is solvable for sure, but I think that a good solution would be pretty architecturally invasive and requires more consensus before pushing it through.

@evaporei
Copy link
Author

evaporei commented Jan 4, 2019

Ok... So should we close this?

@ashleygwilliams
Copy link
Member

@otaviopace yeah i think so- thank you so much for your efforts (and amazing emoji usage)- lemme know if there's anything else you'd like to work on! would be happy to help you get started :)

@evaporei evaporei deleted the add-color-to-cargo-command branch January 4, 2019 21:57
@evaporei
Copy link
Author

evaporei commented Jan 4, 2019

Great, I will definetly see if I can help with something 😄

@ashleygwilliams ashleygwilliams removed this from the 0.6.0 milestone Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashleygwilliams ashleygwilliams ashleygwilliams left review comments

@alexcrichton alexcrichton Awaiting requested review from alexcrichton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@evaporei @alexcrichton @ashleygwilliams