-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: Translatable diagnostics #19545
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
Conversation
This is a proof of concept, so please give me feedback to continue |
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.
Feedback as an outsider to RA: If you're going to use fluent, it could be interesting seeing if rustc_fluent_macro
could be added to auto-publish and have static checking support. Also, adding two whole modules just for fluent is probably not the most ergonomic solution.
@@ -10,8 +10,8 @@ pub(crate) fn await_outside_of_async( | |||
let display_range = | |||
adjusted_display_range(ctx, d.node, &|node| Some(node.await_token()?.text_range())); | |||
Diagnostic::new( | |||
crate::DiagnosticCode::RustcHardError("E0728"), | |||
format!("`await` is used inside {}, which is not an `async` context", d.location), | |||
crate::DiagnosticCode::RustcHardError(fluent_files::await_outside_of_async_code), |
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.
Rustc errors do not change with translations
Some concerns I brought up in Discord, I saw the PR before reading that it's meant to be forked:
The safest bet for implementing multilanguage solutions is letting the community create language packs and linking I'm not sure if Rust wants to have 5-10 forks with variable levels of maintability (without accounting for a possible vector of attack, because we'll have to warn that they're user maintained and watch them closely to avoid someone making a fork that mines bitcoin in user's machines) |
After discussion with the team, we feel like we are not ready to accept this change, at least for now. Our diagnostics infrastructure is still pretty immature, and this will add complexity and maintenance burden. I'm going to close this for now. Thanks for your effort and interest! |
@ChayimFriedman2 Thank you for taking this into consideration. I understand your concerns about added complexity, but I actually believe this change would improve rather than harm maintainability—by centralizing the diagnostic messages, it will help mature the system over time. Although the PR highlights translations, that was only one visible aspect; the real win is having a single, consistent declaration site for all diagnostics. Our group (@RustLangES) is committed to keeping a Spanish localization of these messages, which is precisely why we proposed refining the way they’re declared. We’d love to be part of the next team discussion on this topic—could you let us know when and where that will take place? Thanks again for your time and feedback. |
We discussed that on Zulip. Privately, but feel free to open a public topic on this (#t-compiler/rust-analyzer). I'll try to answer your arguments, though: adding code always adds maintenance effort. And this not just adds code, but changes the way we create new diagnostic, which may negatively impact the experience. Plus, it may hinder the evolution of diagnostics code into something more maintainable. I don't see how this helps maintaining that code. |
This PR introduces infrastructure to make diagnostics in rust-analyzer translatable using Fluent.
Overview
Motivation
Enables future localization of diagnostics, improving accessibility for non-English users. This is not focused on built-in localization, but forks with their own messages.
Next Steps