Skip to content

Pass --color=always through cargo-miri #2243

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
Merged
merged 1 commit into from
Jun 20, 2022
Merged

Pass --color=always through cargo-miri #2243

merged 1 commit into from
Jun 20, 2022

Conversation

saethlin
Copy link
Member

Closes #2037

I just implemented the fix suggested in that issue and it seems to work without issue.

RalfJung
RalfJung reviewed Jun 20, 2022
View reviewed changes
RalfJung
RalfJung reviewed Jun 20, 2022
View reviewed changes
@oli-obk
Copy link
Contributor

oli-obk commented Jun 20, 2022

with ui tests we could actually test this now, too. It's a bit weird to read as a test, but it would mostly be a canary to detect changes anyway.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 20, 2022

@bors r+

@bors
Copy link
Contributor

bors commented Jun 20, 2022

📌 Commit 6cd74ee has been approved by oli-obk

@bors
Copy link
Contributor

bors commented Jun 20, 2022

⌛ Testing commit 6cd74ee with merge b2616ce...

@saethlin
Copy link
Member Author

Btw, I do agree that it would be good to have a test for this, but I've just been poking around in the test-cargo-miri directory and I'm a bit lost.

@bors
Copy link
Contributor

bors commented Jun 20, 2022

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing b2616ce to master...

@bors bors merged commit b2616ce into rust-lang:master Jun 20, 2022
@RalfJung
Copy link
Member

I've just been poking around in the test-cargo-miri directory and I'm a bit lost

Yeah it's somewhat chaotic. :/

@saethlin saethlin mentioned this pull request Jun 20, 2022
Merged
bors added a commit that referenced this pull request Jun 26, 2022
@bors
Auto merge of #2245 - saethlin:color-always, r=RalfJung
9e2dac4 
Actually pass through the request for --color=always

#2243 actually doesn't work 😂

The suggestion to split on `,` was good but `arg` is actually the whole `--json=diagnostic-rendered-ansi,artifacts,future-incompat
`, and of course I didn't test that change locally and we have no test for this in CI.

Therefore, I would like some guidance on making a test for this because I'm going to rely on this working.
@RalfJung
Copy link
Member

RalfJung commented Oct 11, 2022 via email

@oli-obk
Copy link
Contributor

oli-obk commented Oct 11, 2022

I could try adding a ui_test to the cargo-miri crate, not sure what problems will crop up, but ui_test doesn't actually care about what binary it invokes.

@RalfJung
Copy link
Member

This was an old email from months ago that github now added to the issue...

@RalfJung
Copy link
Member

Also this PR has been reverted since: #2283

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

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

Successfully merging this pull request may close these issues.

(cargo-)miri doesn't pass through --color=always?
4 participants
@saethlin @oli-obk @bors @RalfJung