Skip to content

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

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

3 participants