-
-
Notifications
You must be signed in to change notification settings - Fork 941
Support translations in RSS feeds #6317
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
base: main
Are you sure you want to change the base?
Conversation
| req: &HttpRequest, | ||
| context: &web::Data<LemmyContext>, | ||
| ) -> Result<Lang, Error> { | ||
| let jwt = read_auth_token(&req)?; |
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.
Would be good to read Accept-Language HTTP header as well for the language.
…into rss-translations-support
|
You dont need to wait for the translations to be merged. Instead do |
| }; | ||
|
|
||
| for client_lang in client_langs.ranked() { | ||
| let lang_id = match client_lang.item() { |
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.
| let lang_id = match client_lang.item() { | |
| match client_lang.item() | |
| .map(|l| LanguageId::new(lang.primary_language())) | |
| .map(|l| Lang::from_language_id(&lang_id)) { | |
| Some(l) => return l, | |
| None => continue, | |
| } |
No need for multiple match statements this way,
| } | ||
|
|
||
| fn negotiate_lang(req: &HttpRequest) -> Option<Lang> { | ||
| let client_langs = match req.headers().get(ACCEPT_LANGUAGE) { |
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.
No need for match, use req.headers().get(ACCEPT_LANGUAGE)? to return None automatically.
| .split(',') | ||
| .filter_map(|hv| hv.parse().ok()) | ||
| .collect(), | ||
| ), |
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.
Should work with let langs: AcceptLanguage = header.parse()? or similar.
| context: &web::Data<LemmyContext>, | ||
| ) -> Result<Lang, Error> { | ||
| let jwt = read_auth_token(&req)?; | ||
| let lang; |
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.
In rust you can return data from blocks.
So do
let lang = if let Some(jwt) = jwt {
...
lang
}Also what IDE are you using? I recommend using either helix or vim with rust-anaylzer, or jetbrains.
| context: &web::Data<LemmyContext>, | ||
| ) -> Result<Lang, Error> { | ||
| let jwt = read_auth_token(&req)?; | ||
| let lang; |
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.
In rust you can return data from blocks.
So do
let lang = if let Some(jwt) = jwt {
...
}Also what IDE are you using? I recommend using either helix or vim with rust-anaylzer, or jetbrains.
| .filter_map(|hv| hv.parse().ok()) | ||
| .collect(), | ||
| ), | ||
| None => return None, |
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.
Don't return from blocks like this. You should probably refactor this entire thing as an if let block:
if let Some(client_langs) = req.headers().get(ACCEPT_LANGUAGE) {
Issue: #5365
Pipeline build is failing because this PR is dependent on: LemmyNet/lemmy-translations#246
Nortification feed testing:
Modlog feed testing:
Test fallback language: