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

Grundlagen für I18N #136

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Grundlagen für I18N #136

wants to merge 10 commits into from

Conversation

MHajoha
Copy link
Member

@MHajoha MHajoha commented Feb 24, 2025

  • Zumeist muss ein Paket während es importiert wird seine .mo-Dateien laden. Dafür ist es nötig, dass es schon während der Initialisierung in get_qpy_environment().packages liegt. Das ist jetzt der Fall. Uninitialisierte Pakete können stattdessen an dem neuen Feld Package.state erkannt werden.
  • Im Manifest ist languages nun benötigt und muss mindestens einen Eintrag haben. Das ist sinnvoll, damit wir bei Paketen, die nicht lokalisiert sind, die Sprache kennen. Per Konvention ist der erste (und dann einzige) Eintrag von languages dann die Sprache, in der die unübersetzten Strings, also die Argumente zu gettext & co., geschrieben sind.
  • Ein neues Protocol TranslatableString (das NB auch von str erfüllt wird) ist notwendig, weil gettext auch außerhalb eines Requests (insb. während in der FormModel-Definition) aufgerufen wird. TranslatableString wird im SDK dann implementiert und übersetzt den String erst, wenn er wirklich gebraucht wird. Ich hatte hier zunächst auf collections.UserString gesetzt, das ist aber klobiger und bringt das Problem, dass format() -> str ist, also _(...).format(...) nicht funktioniert

Issue: questionpy-org/questionpy-sdk#146
SDK-PR: questionpy-org/questionpy-sdk#154

This is necessary to allow a package to get its Package instance during its own initialization. To allow distinction of initialized and uninitialized packages, they now have a 'state' property.
Copy link
Contributor

@MartinGauk MartinGauk left a comment

Choose a reason for hiding this comment

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

Ich denke wir sollten in questionpy_common lieber kein TranslatableString unterstützen, um das Interface zwischen Worker/Server und Server/LMS leichter verständlich zu halten. Die Worker müssen direkt fertig übersetze Strings zurückgeben.

@MHajoha
Copy link
Member Author

MHajoha commented Mar 3, 2025

Ich denke wir sollten in questionpy_common lieber kein TranslatableString unterstützen, um das Interface zwischen Worker/Server und Server/LMS leichter verständlich zu halten. Die Worker müssen direkt fertig übersetze Strings zurückgeben.

Finde ich auch erstrebenswert, würde hier aber halt duplizierte Form-Element-Models (In Server/Common mit str, und im SDK mit TranslatableString) voraussetzen. Alternativ könnte man mit Generics experimentieren (TextInputElement[TranslatableString]) oder die Form im Server komplett opak lassen (get_options_form(self, question_state: str | None) -> tuple[JsonValue, dict[str, JsonValue]]).

@MartinGauk
Copy link
Contributor

Okay, da gefällt mir die jetzige Variante doch am besten. Dann würde ich TranslatableString nur aus dem Attempt raushalten wollen. Da die Objekte zu den Models eh nicht direkt erstellt werden sondern von questionpy._wrappers._question, können die Übersetzungen dort beim Export stattfinden. Andererseits frage ich mich gerade, ob wir überhaupt TranslatableString als Rückgaben im Attempt akzeptieren sollten. Das kann ja praktisch nur bei _(defer=False) auftreten, oder?

@MHajoha
Copy link
Member Author

MHajoha commented Mar 4, 2025

Andererseits frage ich mich gerade, ob wir überhaupt TranslatableString als Rückgaben im Attempt akzeptieren sollten. Das kann ja praktisch nur bei _(defer=False) auftreten, oder?

Fände ich auch besser (und hatte ich zunächst versucht), leider weiß MyPy das aber nicht. Ohne defer=False ist der Rückgabewert str | TranslatableString. Folgendes würde also korrekterweise von MyPy als Fehler erkannt werden:

    @property
    def general_feedback(self) -> str:
        # error: Incompatible return value type (got "str | TranslatableString", expected "str")  [return-value]
        return _("Programmatic translation!")

Es spricht allerdings in der Tat nichts dagegen, die Übersetzung in _export_attempt zu machen, das werde ich ändern 👍

@MHajoha MHajoha requested a review from MartinGauk March 4, 2025 16:48
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.

2 participants