Merge pull request #2833 from zas/revisit_plugin_oo

Revisit plugin/registry, refactor for more object-oriented approach

Author: zas

picard/git/ops.py

@@ -30,6 +30,7 @@
     GitStatusFlag,
 )
 from picard.git.factory import git_backend
+from picard.git.ref_utils import get_ref_type
 
 
 def clean_python_cache(directory):
@@ -118,12 +119,12 @@ def validate_ref(url, ref, uuid=None, registry=None):
         # For registry plugins with versioning_scheme, validate against version tags
         if registry and uuid:
             registry_plugin = registry.find_plugin(uuid=uuid)
-            if registry_plugin and registry_plugin.get('versioning_scheme'):
+            if registry_plugin and registry_plugin.versioning_scheme:
                 # Import here to avoid circular dependency
                 from picard.plugin3.refs_cache import RefsCache
 
                 refs_cache = RefsCache(registry)
-                pattern = refs_cache.parse_versioning_scheme(registry_plugin['versioning_scheme'])
+                pattern = refs_cache.parse_versioning_scheme(registry_plugin.versioning_scheme)
                 if pattern and pattern.match(ref):
                     # It's a version tag - fetch and check
                     version_tags = []
@@ -136,8 +137,8 @@ def validate_ref(url, ref, uuid=None, registry=None):
                     return True
 
             # For registry plugins with explicit refs list
-            if registry_plugin and registry_plugin.get('refs'):
-                ref_names = [r['name'] for r in registry_plugin['refs']]
+            if registry_plugin and registry_plugin.refs:
+                ref_names = [r['name'] for r in registry_plugin.refs]
                 if ref not in ref_names:
                     raise PluginRefNotFoundError(uuid, ref)
                 return True
@@ -178,29 +179,17 @@ def check_ref_type(repo_path, ref=None):
         try:
             backend = git_backend()
             repo = backend.create_repository(repo_path)
-            references = repo.get_references()
 
             if ref:
-                # Check if ref exists in standard locations first
-                if f'refs/tags/{ref}' in references:
+                # Use robust reference type detection
+                ref_type, resolved_ref = get_ref_type(repo, ref)
+                if ref_type == 'tag':
                     return 'tag', ref
-                if f'refs/heads/{ref}' in references:
-                    return 'branch', ref
-                if f'refs/remotes/origin/{ref}' in references:
+                elif ref_type in ('local_branch', 'remote_branch'):
                     return 'branch', ref
-
-                # Not found in standard refs, try to resolve it
-                try:
-                    obj = repo.revparse_single(ref)
-                    from picard.git.backend import GitObjectType
-
-                    if obj.type == GitObjectType.COMMIT:
-                        return 'commit', ref
-                    elif obj.type == GitObjectType.TAG:
-                        return 'tag', ref
-                    else:
-                        return None, ref
-                except KeyError:
+                elif ref_type == 'commit':
+                    return 'commit', ref
+                else:
                     return None, ref
             else:
                 # Check current HEAD state

picard/git/ref_utils.py

@@ -0,0 +1,77 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2025 Laurent Monin
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+"""Git reference utilities for robust reference type detection."""
+
+
+def get_ref_type(repo, ref):
+    """Determine the type of a git reference.
+
+    Args:
+        repo: Git repository object
+        ref: Reference name to check
+
+    Returns:
+        tuple: (ref_type, resolved_ref) where ref_type is one of:
+               'tag', 'local_branch', 'remote_branch', 'commit', 'unknown'
+    """
+    if not ref:
+        return 'unknown', ref
+
+    try:
+        # Get all references from the repository
+        references = repo.get_references()
+
+        # Check exact matches first
+        if f'refs/tags/{ref}' in references:
+            return 'tag', f'refs/tags/{ref}'
+        if f'refs/heads/{ref}' in references:
+            return 'local_branch', f'refs/heads/{ref}'
+        if f'refs/remotes/{ref}' in references:
+            return 'remote_branch', f'refs/remotes/{ref}'
+        if f'refs/remotes/origin/{ref}' in references:
+            return 'remote_branch', f'refs/remotes/origin/{ref}'
+
+        # Check if ref is already a full reference
+        if ref in references:
+            if ref.startswith('refs/tags/'):
+                return 'tag', ref
+            elif ref.startswith('refs/heads/'):
+                return 'local_branch', ref
+            elif ref.startswith('refs/remotes/'):
+                return 'remote_branch', ref
+
+        # Try to resolve as commit hash
+        try:
+            repo.revparse_single(ref)
+            return 'commit', ref
+        except KeyError:
+            pass
+
+    except Exception:
+        # If we can't get references, fall back to string analysis
+        if ref.startswith('refs/tags/'):
+            return 'tag', ref
+        elif ref.startswith('refs/heads/'):
+            return 'local_branch', ref
+        elif ref.startswith('refs/remotes/') or ref.startswith('origin/'):
+            return 'remote_branch', ref
+
+    return 'unknown', ref

picard/plugin3/api.py

@@ -273,6 +273,11 @@ def _install_qt_translator(self) -> None:
             self._qt_translator._current_locale = self.get_locale()
             self._tagger._qt_translators.add_translator(self._qt_translator)
 
+            # Only emit signal if application is already running (not during startup)
+            # This ensures UI retranslation when plugins are installed dynamically
+            if hasattr(self._tagger, 'window') and self._tagger.window:
+                self._tagger._qt_translators_updated.emit()
+
     def _remove_qt_translator(self) -> None:
         """Remove Qt translator for .ui file translations."""
         if not self._qt_translator:
@@ -281,6 +286,27 @@ def _remove_qt_translator(self) -> None:
         self._tagger._qt_translators.remove_translator(self._qt_translator)
         self._qt_translator = None
 
+    def reload_translations(self) -> None:
+        """Reload translations and reinstall Qt translator.
+
+        Used when plugin is updated to refresh translations without recreating API instance.
+        """
+        # Clear existing translations
+        self._translations.clear()
+
+        # Remove old Qt translator
+        self._remove_qt_translator()
+
+        # Reload translations from disk
+        self._load_translations()
+
+        # Reinstall Qt translator with new translations
+        self._install_qt_translator()
+
+        # Emit signal to trigger Qt translator reinstall and UI retranslation
+        # This is only called during plugin updates, not normal installation
+        self._tagger._qt_translators_updated.emit()
+
     def _is_valid_locale(self, locale: str) -> bool:
         """Check if locale string is valid (basic sanity check).
 
@@ -402,6 +428,9 @@ def _load_translation_file_for_locale(self, locale: str) -> bool:
             data = self._load_translation_file(file_path, format, locale)
             if data:
                 self._translations[locale] = data
+                self._logger.debug(
+                    "Loaded %d translations for locale '%s': %s", len(data), locale, list(data.keys())[:10]
+                )
                 return True
         return False
 
@@ -531,20 +560,31 @@ def tr(self, key: str, text: str | None = None, **kwargs) -> str:
             # Try exact locale match (e.g., de_DE)
             if locale in self._translations and key in self._translations[locale]:
                 result = self._translations[locale][key]
+                self._logger.debug("tr() found exact locale match: '%s' -> '%s'", key, result)
             else:
                 # Try language without region (e.g., de from de_DE)
                 lang = locale.split('_')[0]
                 if lang in self._translations and key in self._translations[lang]:
                     result = self._translations[lang][key]
+                    self._logger.debug("tr() found language match: '%s' -> '%s'", key, result)
+                else:
+                    # Try source locale as fallback
+                    if self._source_locale in self._translations and key in self._translations[self._source_locale]:
+                        result = self._translations[self._source_locale][key]
+                        self._logger.debug("tr() found source locale match: '%s' -> '%s'", key, result)
+                    else:
+                        self._logger.debug("tr() no translation found for key '%s' in any locale", key)
 
         # Fall back to text parameter or key
         if result is None:
             result = text if text is not None else key
+            self._logger.debug("tr() using fallback: '%s' -> '%s'", key, result)
 
         # Apply placeholder substitution
         if kwargs:
             result = result.format(**kwargs)
 
+        self._logger.debug("tr() final result: '%s' -> '%s'", key, result)
         return result
 
     def trn(self, key: str, singular: str | None = None, plural: str | None = None, n: int = 0, **kwargs) -> str:
@@ -587,6 +627,15 @@ def trn(self, key: str, singular: str | None = None, plural: str | None = None,
                         result = trans[plural_form]
                     elif isinstance(trans, dict) and 'other' in trans:
                         result = trans['other']
+                else:
+                    # Try source locale as fallback
+                    if self._source_locale in self._translations and key in self._translations[self._source_locale]:
+                        trans = self._translations[self._source_locale][key]
+                        source_plural_form = get_plural_form(self._source_locale, n)
+                        if isinstance(trans, dict) and source_plural_form in trans:
+                            result = trans[source_plural_form]
+                        elif isinstance(trans, dict) and 'other' in trans:
+                            result = trans['other']
 
         # Fall back to singular/plural parameters
         if result is None:

picard/plugin3/asyncops/registry.py

@@ -44,17 +44,18 @@ def fetch_registry(self, callback, use_cache=True):
             use_cache: Whether to use cached registry
         """
         # Check cache first
-        if use_cache and self._registry._registry_data:
-            callback(OperationResult(success=True, result=self._registry._registry_data))
+        if use_cache and self._registry.is_registry_loaded():
+            callback(OperationResult(success=True, result=self._registry.get_raw_registry_data()))
             return
 
         def _on_response(data, reply, error):
             if error:
                 callback(OperationResult(success=False, error=error, error_message=str(error)))
             else:
                 try:
-                    self._registry._registry_data = json.loads(data)
-                    callback(OperationResult(success=True, result=self._registry._registry_data))
+                    registry_data = json.loads(data)
+                    self._registry.set_raw_registry_data(registry_data)
+                    callback(OperationResult(success=True, result=registry_data))
                 except json.JSONDecodeError as e:
                     callback(OperationResult(success=False, error=e, error_message=f'Invalid JSON: {e}'))