Skip to content

fix: restore translate_formatted() method #830

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

Merged
merged 4 commits into from
Mar 6, 2025
Merged

fix: restore translate_formatted() method #830

merged 4 commits into from
Mar 6, 2025

Conversation

CyanVoxel
Copy link
Member

@CyanVoxel CyanVoxel commented Mar 4, 2025

Summary

This PR restores the use of the translate_formatted() method (now refactored as just format()) that was removed in #817 in order to catch KeyErrors raised from invalid formatting keys being added in the translation files and passed to .format() calls.

Tasks Completed

@CyanVoxel CyanVoxel added Type: Bug Something isn't working as intended Priority: Critical An issue that requires immediate attention Type: Refactor Code that needs to be restructured or cleaned up Type: Translations Modifies translation keys or translation capabilities. Status: Review Needed A review of this is needed labels Mar 4, 2025
@CyanVoxel CyanVoxel added this to the Alpha v9.5.1 milestone Mar 4, 2025
@CyanVoxel CyanVoxel moved this to 🏓 Ready for Review in TagStudio Development Mar 4, 2025
@Computerdores
Copy link
Collaborator

If you end up choosing this PR over #829, I would suggest porting the formatting improvements and renaming translate_formatted to formatted as Translations.translate_formatted seems a bit redundant (I had changed this in #817 as well before removing the method altogether)

Copy link
Collaborator

@Computerdores Computerdores left a comment

Choose a reason for hiding this comment

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

No issues besides what was mentioned before

)
return text

def translate_formatted(self, key: str, **kwargs) -> str:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest renaming this to formatted as outlined in the comment

Copy link
Member Author

Choose a reason for hiding this comment

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

I was looking forward to doing this as well

@@ -27,6 +27,21 @@ def change_language(self, lang: str):
self._lang = lang
self._strings = self.__get_translation_dict(lang)

def __format(self, text: str, **kwargs) -> str:
Copy link
Collaborator

@Computerdores Computerdores Mar 4, 2025

Choose a reason for hiding this comment

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

I would suggest porting the improvements from #829

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change it to this (plus an import for defaultdict at the top):

    def __format(self, text: str, *args, **kwargs) -> str:
        try:
            return text.format(*args, **kwargs)
        except (KeyError, ValueError):
            logger.error(
                "[Translations] Error while formatting translation.",
                text=text,
                kwargs=kwargs,
                language=self._lang,
            )
            params: defaultdict = defaultdict(lambda: "{missing_key}")
            params.update(kwargs)
            return super().format_map(params)

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in 499b548

@CyanVoxel CyanVoxel moved this from 🏓 Ready for Review to 👀 In review in TagStudio Development Mar 4, 2025
@Computerdores Computerdores moved this from 👀 In review to 🍃 Pending Merge in TagStudio Development Mar 6, 2025
@Computerdores Computerdores removed the Status: Review Needed A review of this is needed label Mar 6, 2025
@CyanVoxel CyanVoxel merged commit 7e75087 into main Mar 6, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from 🍃 Pending Merge to ✅ Done in TagStudio Development Mar 6, 2025
@CyanVoxel CyanVoxel deleted the fix-827 branch March 6, 2025 21:06
csponge pushed a commit to csponge/TagStudio that referenced this pull request Mar 10, 2025
…ev#830)

* fix: restore `translate_formatted()` method

* fix: set "Create & Add" button text

* refactor: rename "translate_formatted" to "format"

* translations: replace invalid format key names with "{unknown_key}"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Critical An issue that requires immediate attention Type: Bug Something isn't working as intended Type: Refactor Code that needs to be restructured or cleaned up Type: Translations Modifies translation keys or translation capabilities.
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Bug]: Switching translations causes error on program start
2 participants