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

Fixed #35306 -- Updated 5.0 Docs #18028

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

lufafajoshua
Copy link
Contributor

@lufafajoshua lufafajoshua commented Mar 28, 2024

Trac ticket number

ticket-35306

Branch description

Updated documentation on localization formats usage and precedence.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • For UI changes, I have attached screenshots in both light and dark modes.

Updated documentation on localization formats usage
and precedence.
@nessita
Copy link
Contributor

nessita commented Mar 28, 2024

@lufafajoshua thank you for your work.

When this PR is ready for review, please update the checklist from the PR description, and don't forget to set the proper ticket flags for this to be listed in the branches needing review.

:doc:`/topics/i18n/index`.

:setting:`USE_I18N` must be active for this setting to have any effect.
A string representing the fallback language code for this installation.
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we can add fallback here. The paragraph below explains in which case it is used as a fallback language, and when it is the language served to all users.


* If the locale middleware isn't in use, it decides which translation is served
to all users.
* If the locale middleware is active, it provides a fallback language in case the
user's preferred language can't be determined or is not supported by the
website. It also provides the fallback translation when a translation for a
given literal doesn't exist for the user's preferred language.
* If localization is explicitly disabled via the :tfilter:`localize` and
:tfilter:`unlocalize` template filters or tags. see
:ref:`controlling localization in templates <topic-l10n-templates>`.

Copy link
Member

Choose a reason for hiding this comment

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

This sentence is not complete. However, as a non-native speaker, I'll happily defer choosing a better formulation to someone else.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, "see" needs to be capitalized and it would be good to end with "for [insert something]".
You can just say "for more details" but you might want to be more specific like "for further explanation on [insert something]."
Rough example:

See :ref:`controlling localization in templates <topic-l10n-templates>` for more details.

@@ -60,6 +60,9 @@ to be machine-readable, you will always want unlocalized values. You
may also want to use localization in selected templates, rather than
using localization everywhere.

The format used is the one from the settings and under normal circumstances
the locale dictated format has a higher precedence and will be applied instead.
Copy link
Member

Choose a reason for hiding this comment

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

What format? Which settings? Sorry, but I don't think this adds much clarity to these docs.

Maybe something like (once again I'm not a skilled English writer):
When localization is disabled through one of the tag or filter described below, the localization formats used are taken from the general formatting settings (list of settings here?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @claudep , thanks for your review and response on this, Just for clarity, is it okay to pass a reference to the globalization settings as the general formatting settings instead of listing them.

Copy link
Member

Choose a reason for hiding this comment

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

Good idea for this link! Now that I see that list, maybe it would be nice to distinguish in that list the settings that may overriding by locale formats, and those that don't, and then suggest (maybe in a different PR) separating that list in two sub-lists. Then we would have a perfect section to link to in this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds like we still want to do this, it can be a separate commit or another PR 👍

Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

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

Thank you for this @lufafajoshua ⭐ I have a few comments
Can you also update the PR title to better represent what this PR is trying to acheive?

Comment on lines -1841 to +1844
A string representing the language code for this installation. This should be in
standard :term:`language ID format <language code>`. For example, U.S. English
is ``"en-us"``. See also the `list of language identifiers`_ and
:doc:`/topics/i18n/index`.

:setting:`USE_I18N` must be active for this setting to have any effect.
A string representing the language code for this installation.
This should be in standard :term:`language ID format <language code>`.
For example, U.S. English is ``"en-us"``. See also the
`list of language identifiers`_ and :doc:`/topics/i18n/index`.
Copy link
Contributor

@sarahboyce sarahboyce May 5, 2024

Choose a reason for hiding this comment

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

This is mostly the same, can you revert the changes to the line length?

Comment on lines +1854 to +1857
* If localization is explicitly disabled via the :tfilter:`localize` and
:tfilter:`unlocalize` template filters or tags, it provides fallback
localization formats which will be applied instead. see
:ref:`controlling localization in templates <topic-l10n-templates>`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think as we're referencing explicitly disabling localization, we should reference

:tfilter:`unlocalize`

and

:ttag:`{% localize off %} <localize>`

I think it might be wrong to reference

:tfilter:`localize`

when talking about disabling localization


* If the locale middleware isn't in use, it decides which translation is served
to all users.
* If the locale middleware is active, it provides a fallback language in case the
user's preferred language can't be determined or is not supported by the
website. It also provides the fallback translation when a translation for a
given literal doesn't exist for the user's preferred language.
* If localization is explicitly disabled via the :tfilter:`localize` and
:tfilter:`unlocalize` template filters or tags. see
:ref:`controlling localization in templates <topic-l10n-templates>`.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, "see" needs to be capitalized and it would be good to end with "for [insert something]".
You can just say "for more details" but you might want to be more specific like "for further explanation on [insert something]."
Rough example:

See :ref:`controlling localization in templates <topic-l10n-templates>` for more details.

@@ -60,6 +60,9 @@ to be machine-readable, you will always want unlocalized values. You
may also want to use localization in selected templates, rather than
using localization everywhere.

The format used is the one from the settings and under normal circumstances
the locale dictated format has a higher precedence and will be applied instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

It sounds like we still want to do this, it can be a separate commit or another PR 👍

@@ -3666,6 +3668,9 @@ Forms
* :setting:`FORM_RENDERER`
* :setting:`FORMS_URLFIELD_ASSUME_HTTPS`


.. _globalization-settings:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a new unused reference, I believe the start of implementing this comment.

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