Skip to content
Open
Show file tree
Hide file tree
Changes from 59 commits
Commits
Show all changes
80 commits
Select commit Hold shift + click to select a range
63e4218
Add ui to manage custom columns
knguyen1 Aug 21, 2025
02261a5
Fix failing tests
knguyen1 Aug 21, 2025
895ef3f
Use `IntEnum` and `HEADERS` dict; do not hardcode vals
knguyen1 Aug 26, 2025
5c56759
No need to import in try-except
knguyen1 Aug 26, 2025
dd09a0a
Remove readme's in code tree
knguyen1 Aug 26, 2025
6ffe167
Remove blind catch; log debug msg
knguyen1 Aug 27, 2025
11b7163
Do not hardcode ints; use screaming constants
knguyen1 Aug 27, 2025
79e9363
Use `add_to` instead of `add_to_file_view`, `add_to_album_view`, etc.
knguyen1 Aug 27, 2025
169045c
Refactor `validate` to flatten results via a comprehension
knguyen1 Aug 27, 2025
cb53ff4
Rename `upper` -> `align`
knguyen1 Aug 27, 2025
ae5dd08
Use generic `Iterable`, not `list`
knguyen1 Aug 27, 2025
e6b1f5f
Use `tuple` in place of list
knguyen1 Aug 27, 2025
264aa1d
Add dirty/cache mechanism for O(1) in `get_by_key`
knguyen1 Aug 27, 2025
2ba8687
Remove `insert_after_key` concept; BLE001 fix
knguyen1 Sep 1, 2025
f3e04b6
Apply live delete/create to columns
knguyen1 Sep 1, 2025
77d8e8f
Fix bug with field type custom column
knguyen1 Sep 1, 2025
25fb12d
Fix live column updates; remove `is_default` concept
knguyen1 Sep 1, 2025
1b31fa9
Provide a way to enter key; deduplicate key
knguyen1 Sep 1, 2025
facd6a2
Should import directly
knguyen1 Sep 1, 2025
88361ef
Add user-facing tooltips to `expression_dialog.py`
knguyen1 Sep 1, 2025
0b5da55
Use try...except..else pattern
knguyen1 Sep 1, 2025
60745e0
Make `left`/`right` choices translatable
knguyen1 Sep 1, 2025
5cf7992
Make show-in-view selection generic
knguyen1 Sep 1, 2025
bc31743
`FieldReferenceProvider` needs a Callable column guard
knguyen1 Sep 1, 2025
340d513
Rename `Field Name` -> `Column Title`
knguyen1 Sep 1, 2025
82b93cb
Refactor buttons for consistency
knguyen1 Sep 2, 2025
e8e611b
ui: single dialog custom columns manager; use `ScriptTextEdit`
knguyen1 Sep 2, 2025
056c111
fix: Inputs should reset on add
knguyen1 Sep 2, 2025
1cc7093
Apply better abstractions/decoupling for SRP
knguyen1 Sep 2, 2025
69b4f56
Scripts should be evaluated for errors
knguyen1 Sep 2, 2025
4ed5aeb
Switch over to use `ColumnSpecValidator` from `validation.py`
knguyen1 Sep 2, 2025
fe4753a
Massive refactor: SRP/DRY/SOC, etc; rewire flows
knguyen1 Sep 3, 2025
eccaf24
Add unit tests
knguyen1 Sep 3, 2025
68369b2
Add live spec check
knguyen1 Sep 3, 2025
546d933
Add help button
knguyen1 Sep 3, 2025
3d99541
`Manage Columns…` -> `Manage Custom Columns…` context menu
knguyen1 Sep 3, 2025
aafaf02
Fix failing tests; QT objects need to be mocked
knguyen1 Sep 3, 2025
2076a95
Fix failing tests
knguyen1 Sep 3, 2025
3ed794a
Update picard/ui/itemviews/custom_columns/manager_dialog.py
knguyen1 Sep 3, 2025
70c26f2
Fix rdswift's bug: Uncommitted changes
knguyen1 Sep 3, 2025
4774ba0
Add 'stage changes' clarification
knguyen1 Sep 3, 2025
f7ba2f4
Move `refresh_all_views` to event bus
knguyen1 Sep 3, 2025
6a1aed0
Make col ids guid; fix bug with delayed commit
knguyen1 Sep 4, 2025
f0450fd
Fix failing tests
knguyen1 Sep 4, 2025
cedbbb4
Minor: must click `Add` to add initially; dup/del disabled unless has…
knguyen1 Sep 4, 2025
eed6a77
refactor: move `SpecListModel` to own module
knguyen1 Sep 4, 2025
a3b51de
refactor: Make `manager_dialog.py` lighter
knguyen1 Sep 4, 2025
d3617e0
Add ability to pick a sorting algo
knguyen1 Sep 4, 2025
28fba33
Add NAT sorting; fix failing tests
knguyen1 Sep 4, 2025
48a2bff
Add natural sorter; fix failing tests
knguyen1 Sep 4, 2025
e9e0b81
Remove 'reverse' sorter from UI
knguyen1 Sep 4, 2025
e552137
Alias the sort_key import
knguyen1 Sep 4, 2025
e0e779f
Need to skip these tests on MACOS
knguyen1 Sep 4, 2025
4e2aa24
Natural sorting behaviour is different on Linux
knguyen1 Sep 4, 2025
b45413d
Formatting review by rdswift
knguyen1 Sep 4, 2025
74b22ca
PR review change requests from zas
knguyen1 Sep 4, 2025
5a6180c
Rename file refactor -> decoupling
knguyen1 Sep 4, 2025
f19cdfa
Remove `Save Changes` button; auto stage changes
knguyen1 Sep 4, 2025
549ae93
Default col name and focus on add; fix bug invalid spec closing window
knguyen1 Sep 5, 2025
f2752a2
Add should create a placeholder spec
knguyen1 Sep 5, 2025
74ede00
Newly visible columns should have a checkbox
knguyen1 Sep 5, 2025
e3c2c0c
Newly added custom columns should also trigger data refresh
knguyen1 Sep 5, 2025
6728330
Relax key rule; mark and translate errors
knguyen1 Sep 6, 2025
d280936
Remove `Make It So!` button from dialog
knguyen1 Sep 6, 2025
9c0d578
Updates from zas' code review
knguyen1 Sep 6, 2025
855223e
Fix failing tests
knguyen1 Sep 6, 2025
0bf96ec
Suppress mising attr `ClusterList.column` log
knguyen1 Sep 6, 2025
1bb9012
Rename `Add` -> `New`
knguyen1 Sep 6, 2025
12542ea
Allow highlighting multiple and deleting
knguyen1 Sep 6, 2025
01e3fc8
Enhance validation dialog+interaction
knguyen1 Sep 6, 2025
4f5c139
Make expressions optional, show warning
knguyen1 Sep 6, 2025
e164f54
Silently fix key errors
knguyen1 Sep 6, 2025
8aa756b
Unchecking views should de-register col from view
knguyen1 Sep 6, 2025
3f33d18
Set focus on title when `New` is clicked
knguyen1 Sep 6, 2025
9576f39
Use `tuple`; remove redundant code
knguyen1 Sep 6, 2025
4b73418
Update shared.py
knguyen1 Sep 7, 2025
7cdbb3a
`ValidationReport.summary` should be translated
knguyen1 Sep 7, 2025
e3cb29e
Fix bug with eval of regular vs. `~` tilde slugs
knguyen1 Sep 7, 2025
e17a2bc
Fix crash issue due to non-float-parseable values returning `QCollato…
knguyen1 Sep 7, 2025
4d05b75
Do not show hardcoded values for cluster/noncluster
knguyen1 Sep 9, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions picard/ui/itemviews/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,14 +414,13 @@ def update_colums_text(self, color=None, bgcolor=None):
else:
if column.align == ColumnAlign.RIGHT:
self.setTextAlignment(i, QtCore.Qt.AlignmentFlag.AlignRight | QtCore.Qt.AlignmentFlag.AlignVCenter)
# Support custom columns with provider evaluation
try:
if isinstance(column, CustomColumn):

if isinstance(column, CustomColumn):
try:
self.setText(i, column.provider.evaluate(self.obj))
continue
except Exception: # noqa: BLE001
# Fallback to default behavior
pass
except (AttributeError, TypeError, ValueError, KeyError, NotImplementedError) as exc:
log.debug("Custom column '%s' evaluate failed: %r", column.key, exc)
continue
self.setText(i, self.obj.column(column.key))


Expand Down
44 changes: 42 additions & 2 deletions picard/ui/itemviews/basetreeview.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
from picard.ui.collectionmenu import CollectionMenu
from picard.ui.enums import MainAction
from picard.ui.filter import Filter
from picard.ui.itemviews.events import header_events
from picard.ui.itemviews.match_quality_column import MatchQualityColumn, MatchQualityColumnDelegate
from picard.ui.ratingwidget import RatingWidget
from picard.ui.scriptsmenu import ScriptsMenu
Expand Down Expand Up @@ -176,6 +177,10 @@ def __init__(self, columns, window, parent=None):
self.setAccessibleDescription(_(self.DESCRIPTION))
self.tagger = QtCore.QCoreApplication.instance()
self.window = window

# Subscribe to header update events
header_events.headers_updated.connect(self._on_header_updated)

# Should multiple files dropped be assigned to tracks sequentially?
self._move_to_multi_tracks = True

Expand Down Expand Up @@ -387,9 +392,26 @@ def save_state(self):
config.persist[self.header_state] = state
config.persist[self.header_locked] = header.is_locked

def _set_header_labels(self, update_column_count=False):
"""Set header labels based on current columns.

Parameters
----------
update_column_count : bool, optional
Whether to update the column count to match the number of columns,
by default False.
"""
from picard.ui.columns import Columns as _Columns

cols = self.columns
if isinstance(cols, _Columns):
if update_column_count:
self.setColumnCount(len(cols))
labels = tuple(_(c.title) for c in cols)
self.setHeaderLabels(labels)

def restore_default_columns(self):
labels = tuple(_(c.title) for c in self.columns)
self.setHeaderLabels(labels)
self._set_header_labels(update_column_count=False)

header = self.header()
header.setStretchLastSection(True)
Expand All @@ -407,7 +429,25 @@ def restore_default_columns(self):

self.sortByColumn(-1, QtCore.Qt.SortOrder.AscendingOrder)

def _refresh_header_labels(self):
"""Refresh header labels (and count) based on current columns."""
self._set_header_labels(update_column_count=True)

def _on_header_updated(self):
"""Handle global header update events and refresh if applicable."""
from picard.ui.itemviews.custom_columns.shared import get_recognized_view_columns

recognized = set(get_recognized_view_columns().values())
if self.columns not in recognized:
return
self._refresh_header_labels()

def _init_header(self):
# Load any persisted user-defined custom columns before header setup
from picard.ui.itemviews.custom_columns.storage import load_persisted_columns_once
Copy link
Collaborator

Choose a reason for hiding this comment

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

You may have already addressed this, but is there a reason this import isn't at the start of the module?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to avoid circular import and delays the import until we actually need it. To resolve, we could move load_persisted_columns_once to a module higher, e.g. picard/ui/mainwindow.py, picard/ui/itemviews/__init__.py, but at the time, I didn't want to touch modules unrelated to this PR.

The import chain creates a circular dependency:

  1. basetreeview.py imports from picard.ui.itemviews.columns (line 67: from picard.ui.itemviews.columns import ColumnAlign)
  2. custom_columns/storage.py imports from picard.ui.itemviews.custom_columns (line 42-47)
  3. custom_columns/__init__.py imports from custom_columns/registry.py (line 40)
  4. custom_columns/registry.py imports from custom_columns/shared.py (line 32-33)
  5. custom_columns/shared.py has a local import from picard.ui.itemviews.columns (line 183-186)


load_persisted_columns_once()

header = ConfigurableColumnsHeader(self.columns, parent=self)
self.setHeader(header)

Expand Down
4 changes: 4 additions & 0 deletions picard/ui/itemviews/custom_columns/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@
CasefoldSortAdapter,
CompositeSortAdapter,
DescendingCasefoldSortAdapter,
DescendingNaturalSortAdapter,
DescendingNumericSortAdapter,
LengthSortAdapter,
NaturalSortAdapter,
NullsFirstAdapter,
NullsLastAdapter,
NumericSortAdapter,
Expand All @@ -67,6 +69,8 @@
"DescendingCasefoldSortAdapter",
"NumericSortAdapter",
"DescendingNumericSortAdapter",
"NaturalSortAdapter",
"DescendingNaturalSortAdapter",
"LengthSortAdapter",
"ArticleInsensitiveAdapter",
"CompositeSortAdapter",
Expand Down
117 changes: 117 additions & 0 deletions picard/ui/itemviews/custom_columns/column_controller.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
# -*- coding: utf-8 -*-
#
# Picard, the next-generation MusicBrainz tagger
#
# Copyright (C) 2025 The MusicBrainz Team
#
# 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.

"""Column controller facade for custom column management dialogs.

Thin facade layer that reduces complexity in dialog components by
encapsulating validation and specification management logic.

Classes
-------
ColumnController
Facade class that simplifies custom column operations for dialogs.
"""

from typing import Iterable

from picard.ui.itemviews.custom_columns.column_spec_service import ColumnSpecService
from picard.ui.itemviews.custom_columns.spec_list_model import SpecListModel
from picard.ui.itemviews.custom_columns.storage import CustomColumnSpec
from picard.ui.itemviews.custom_columns.validation import (
ColumnSpecValidator,
ValidationContext,
ValidationReport,
)


class ColumnController:
"""Facade for custom column operations in dialog components.

Provides simplified interface for common custom column operations,
delegating to specialized services.

Parameters
----------
spec_service : ColumnSpecService
Service for managing column specifications.
validator : ColumnSpecValidator
Validator for checking specification validity.
"""

def __init__(self, spec_service: ColumnSpecService, validator: ColumnSpecValidator) -> None:
"""Initialize the column controller with required services."""
self._spec_service = spec_service
self._validator = validator

def validate_specs(self, specs: Iterable[CustomColumnSpec]) -> dict[str, ValidationReport]:
"""Validate multiple column specifications.

Returns
-------
dict[str, ValidationReport]
Dictionary mapping specification keys to validation reports.
"""
return self._validator.validate_multiple(specs)

def first_invalid_spec(self, specs: Iterable[CustomColumnSpec]) -> CustomColumnSpec | None:
"""Find the first invalid specification in a collection.

Returns
-------
CustomColumnSpec | None
First invalid specification found, or None if all valid.
"""
reports = self.validate_specs(specs)
for spec, report in reports.items():
if not report.is_valid:
return spec
return None

def first_invalid_spec_report(self, report: dict[str, ValidationReport]) -> tuple[str, ValidationReport] | None:
"""Get first invalid specification with its validation report.

Returns
-------
tuple[str, ValidationReport] | None
(key, validation_report) for first invalid spec, or None.
"""
for key, validation_report in report.items():
if not validation_report.is_valid:
return key, validation_report
return None

def validate_single(self, spec: CustomColumnSpec, existing_keys: set[str]) -> ValidationReport:
"""Validate a single column specification.

Returns
-------
ValidationReport
Report detailing the validation result.
"""
context = ValidationContext(existing_keys=existing_keys)
return self._validator.validate(spec, context)

def apply_all(self, model: SpecListModel) -> None:
"""Apply all specifications from a model.

Deduplicates and persists specifications.
"""
self._spec_service.deduplicate_model_by_keys(model)
self._spec_service.persist_and_register(model.specs())
Loading
Loading