Conversation
frjo
left a comment
There was a problem hiding this comment.
Nice work, must have taken some time!
The error messages we never bothered with before, normal users should never see them hopefully. But no harm in having them translated.
hypha/apply/determinations/forms.py
Outdated
| "unique_together": "You have already created a determination for this submission", | ||
| "unique_together": _( | ||
| "You have already created a determination for this submission" | ||
| ), |
There was a problem hiding this comment.
The forms in this file are old and only used on old installs, in reality only OTF I believe.
I do not think we should translate them, just adding a lot of strings to translate that are not really used.
There was a problem hiding this comment.
I've reverted the changes for that file. The text did appear very OTF-specific when I made the change originally, but I wasn't quite sure so thanks for giving me context!
85bef34 to
35b1949
Compare
35b1949 to
e2bc666
Compare
frjo
left a comment
There was a problem hiding this comment.
Really nice work this! Will be happy to merge it in when we have these minor issues fixed.
| "django-hijack~=3.7.6", | ||
| "django-htmx~=1.27.0", | ||
| "django-nh3~=0.2.1", | ||
| "django-nh3==0.3.1", |
There was a problem hiding this comment.
nh3 is already at 0.3.x in main so this can be removed from the PR.
There was a problem hiding this comment.
This will also remove changes to requirements and uv.lock making for fewer merge conflicts.
(This contribution was sponsored by DigitalHub.sh)
I've split this PR over several commits which are mostly independent of each other. I'm happy to open separate PRs if you think that'd be easier to review.
Explanations of the separate commits
Mark missing strings for translation
After a few unsuccessful attempts at scripting the identification of untranslated strings, I ended up reviewing all Python files manually.
The command I used to identify the relevant files was:
(That's all Python files, but excluding stuff like migrations, settings, ... (and empty files))
This change should have no impact for a site running the English version of Hypha.
Add missing strings for translations (templates)
Similar to the previous commit, I ended up reviewing all templates manually (
git ls-files '*.html'), adding{% trans %},{% blocktrans %}, or_(...)where needed.Again, this change should have no impact.
Add trimmed option to blocktranslate
This removes irrelevant whitespace in source strings, making it a bit nicer.
I used
git grep blocktrans | grep -vw trimmed | grep -v endblocktransto identify uses of{% blocktrans %}without thetrimmedargument, excluding lines that also included{% endblocktrans %}on the same line.This is technically a breaking change, but it only introduces whitespace changes to HTML templates, which is irrelevant.
Use
{% blocktranslate trimmed %}to break down long linesThis one just makes templates a little bit more readable as it breaks very long lines into more manageable pieces.
Again, it can introduce whitespace changes to the HTML, but should not have any effect to the source strings.
Added
verbose_nameandverbose_name_pluralto all modelsSome models already has a translated
verbose_nameorverbose_name_plural, but it was inconsistent (and quite rare). This change adds these attributes to all models.I used
git ls-files '**/models/*.py'andgit ls-files '**/models.py'to find all models, then went through the files manually to add the missingverbose_nameandverbose_name_plural.I decided to omit the plural form for model classes inheriting from
BaseSiteSettingssince it didn't seem to make sense.I also opted to use lower case (like
_("foo bar")for modelFooBar) to match Django's own default behavior, but I usedFoo Barin a few instances where the model already had averbose_namethat used titlecase.Next steps
I ran out of time to complete all my objectives and I've still got some translation-related tasks I wanted to get to. I'm happy to open issues for these if you think they're relevant:
verbose_nameto all declared model fields (and maybe form fields as well). This can probably be scripted somewhat, ideally with a linter that would catch the introducing of future untranslated strings.gettextis used at the module level instead ofgettext_lazyas this could indicate some possible bugs (visible in a multi-language setup where the user is not using the default language).{% trans "Application" %} {{ application_id }} {% trans "updated on" %} {{ date }} {% trans "by" %} {{ author.name }}should be rewritten to have the whole sentence in a singleblocktrans, or when not possible small words likebyshould be given a context because they're likely to be translated differently for different sentences.