-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Reintroduce -Zchalk
with error recommending -Ztrait-solver=chalk
#106514
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
Reintroduce -Zchalk
with error recommending -Ztrait-solver=chalk
#106514
Conversation
r? @nagisa (rustbot has picked a reviewer for you, use r? to override) |
a5ce2da
to
47ecdce
Compare
47ecdce
to
53e46db
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the full refactor like this truly necessary? Could there been a simple if opts.chalk { output_the_diagnostic() }
to start?
I would rather the big refactor be discussed separately as a non-functional-change, rather than bundled with useful changes (like -Zchalk
). Clearly our options parsing code is now quite crufty and it isn’t clear to me if adding more complexity to it is a good tradeoff at the moment.
(also really sorry for the late review!)
}, | ||
Err(ParseError::Removed(in_favor_of)) => early_error( | ||
error_format, | ||
&format!("{outputname} option `{key}` removed in favor of `{in_favor_of}`"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won’t this make the diagnostic here non-translatable? Do we want it to be translatable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just as untranslatable as the existing error message above it. And the inputs are just options, so I don't see why this couldn't be migrated
Yeah, I could probably find somewhere to shove that. I'll put it up as a separate PR, probably much shorter. |
Adds
-Zchalk
back with a helpful error message:cc #106385