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.

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

Fix: Norwegian translations #4647

Merged
merged 3 commits into from Jan 2, 2023
Merged

Conversation

tersor
Copy link
Contributor

@tersor tersor commented Nov 16, 2022

@squidfunk Agree! Removing no could be a breaking change, better to do it in next major release.

@squidfunk
Copy link
Owner

Thanks for providing the updated PR. Unfortunately, not all of my qquestions were answered – does it make sense to have support for no? Does this PR contain actual translations for no or is it a copy of nb? Don't worry about the breaking change, it's my job to take care of it, but I need more information to decide what to do exactly.

@tersor
Copy link
Contributor Author

tersor commented Nov 16, 2022

does it make sense to have support for no?

No, it does not make any sense. The no locale should be avoided since it is a macrolanguage. It is much more preferable to use the code for Norwegian bokmål (variant of the Norwegian language) according to the ISO 639-1 standard, which is nb.

Does this PR contain actual translations for no or is it a copy of nb?

It is a copy of nb to avoid any breaking change. Historically (and unfortunately) no has been used as the locale for Norwegian bokmål. But most modern libraries today use nn and nb exclusively.

@squidfunk
Copy link
Owner

Ok, perfect, thanks for the explanation. Please remove no then, I'll merge this into #4628 then.

@tersor
Copy link
Contributor Author

tersor commented Nov 16, 2022

@squidfunk done!

Copy link
Owner

@squidfunk squidfunk left a comment

Choose a reason for hiding this comment

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

See my comments and our contributing guide.

"meta.comments": "Kommentarar",
"meta.source": "Kjelde",
"search.config.lang": "no",
"nav.title": "Navigasjon",
"search.config.lang": "nn",
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove the changes for search.config.* (here and in the other translation). I'm pretty confident that with those lines present, search will not work. Lunr languages only supports no.

Copy link
Contributor Author

@tersor tersor Nov 17, 2022

Choose a reason for hiding this comment

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

Much appriciated feedback @squidfunk! Sorry for not reading the entire contributing guide, my bad 🙄

I notice in lunr.no.js that it mixes both languages (nn and nb), so I think it is best to define "search.config.lang": "no". Then the search will work too.

src/partials/languages/nn.html Outdated Show resolved Hide resolved
@squidfunk squidfunk mentioned this pull request Nov 27, 2022
25 tasks
@squidfunk
Copy link
Owner

All good! I've merged your changes into the v9 branch, tracked in #4628.

@squidfunk squidfunk mentioned this pull request Dec 7, 2022
34 tasks
@squidfunk squidfunk merged commit bb5e561 into squidfunk:master Jan 2, 2023
@gallaher
Copy link
Contributor

Error message using Norwegian Bokmål:
@squidfunk Using the follwing language code for norwgian nynorsk, nn, works perfectly:
theme:
name: material
language: nn

However, using the code for Norwegian Bokmål, nb, does not work:
theme:
name: material
language: nb

This combination gives an error message and tells me the web page is unavailable. Is there a way to choos Norwegian Bokmål as the language for the page?

Regards,
Martin

@squidfunk
Copy link
Owner

No, nb is the correct code, it should work. Make sure to upgrade to the latest version.

@gallaher
Copy link
Contributor

Thanks, I thought I had installed the latest, but it was v.8.3.3 :-( After updating to the latest revision everything works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants