Skip to content

Commit bbcc7f1

Browse files
committed
refactor: give each kind of setting field its own type
SettingField held the options for every kind of field and a FieldKind saying which applied, so nothing stopped a spin option on a checkbox or a combo without choices, and the form branched on the kind in four places. Each kind is now its own class taking only its own options, with the required ones required, and the form builds, fills and reads each through a matching editor. The crop field's spin_max went with it: the crop picker has no maximum, so it was never read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MTS355x5fJ6RENK1HLYqmX
1 parent 936ba36 commit bbcc7f1

9 files changed

Lines changed: 495 additions & 457 deletions

‎gridplayer/params/defaults_fields.py‎

Lines changed: 225 additions & 281 deletions
Large diffs are not rendered by default.

‎gridplayer/widgets/defaults_form.py‎

Lines changed: 226 additions & 156 deletions
Large diffs are not rendered by default.

‎tests/test_audio_track_defaults.py‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
)
1010
from gridplayer.models.playlist import Playlist
1111
from gridplayer.models.video import Video
12-
from gridplayer.params.defaults_fields import VIDEO_FIELDS, FieldKind
12+
from gridplayer.params.defaults_fields import VIDEO_FIELDS, ComboField, TextField
1313
from gridplayer.params.static import AudioTrackMode
1414
from gridplayer.playlist_settings import (
1515
PlaylistSettings,
@@ -66,8 +66,8 @@ def test_languages_field_follows_the_mode_field():
6666
def test_languages_field_is_a_text_box_with_an_example_in_it():
6767
spec = _spec(LANGUAGES_KEY)
6868

69-
assert spec.kind is FieldKind.TEXT
70-
assert spec.text_placeholder
69+
assert isinstance(spec, TextField)
70+
assert spec.placeholder
7171

7272

7373
def test_text_field_renders_a_line_edit():
@@ -77,7 +77,7 @@ def test_text_field_renders_a_line_edit():
7777
widget = form._widgets[LANGUAGES_KEY]
7878

7979
assert isinstance(widget, QLineEdit)
80-
assert widget.placeholderText() == _spec(LANGUAGES_KEY).text_placeholder
80+
assert widget.placeholderText() == _spec(LANGUAGES_KEY).placeholder
8181

8282

8383
def test_text_field_round_trips_its_value():
@@ -190,9 +190,9 @@ class TestTheAudioTrackDefaultIsAChoiceOfThree:
190190
def test_the_field_offers_them_by_name(self):
191191
spec = _spec(MODE_KEY)
192192

193-
assert spec.kind is FieldKind.COMBO
193+
assert isinstance(spec, ComboField)
194194
assert spec.label == "Audio track"
195-
assert list(spec.combo_values()) == [
195+
assert list(spec.choices()) == [
196196
AudioTrackMode.DEFAULT,
197197
AudioTrackMode.PREFERRED,
198198
AudioTrackMode.DISABLED,

‎tests/test_linked_lists.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1682,7 +1682,7 @@ def test_in_the_lists_section_of_the_playlist_s(self):
16821682

16831683
assert field.playlist_attr == "list_update"
16841684
assert field.label == "Update linked lists"
1685-
assert list(field.combo_values().values()) == [
1685+
assert list(field.choices().values()) == [
16861686
"On playlist load",
16871687
"Every 5 minutes",
16881688
"Every 15 minutes",

‎tests/test_step_sizes.py‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414

1515
from gridplayer.models.playlist import Playlist
1616
from gridplayer.params.actions import ACTIONS
17-
from gridplayer.params.defaults_fields import PLAYLIST_FIELDS, FieldKind
17+
from gridplayer.params.defaults_fields import (
18+
PLAYLIST_FIELDS,
19+
CheckboxField,
20+
FloatSpinField,
21+
)
1822
from gridplayer.params.static import MAX_SCALE, MIN_SCALE
1923
from gridplayer.player.managers.video_blocks import VideoBlocksManager
2024
from gridplayer.playlist_settings import PlaylistSettings, overrides_from_playlist
@@ -102,14 +106,14 @@ def test_seek_steps_are_set_to_the_millisecond(self):
102106
fields = {spec.settings_key: spec for spec in PLAYLIST_FIELDS}
103107

104108
for key in STEP_KEYS[:3]:
105-
assert fields[key].kind is FieldKind.FLOAT_SPIN
106-
assert fields[key].spin_decimals == 3
107-
assert fields[key].spin_min == 0.001
109+
assert isinstance(fields[key], FloatSpinField)
110+
assert fields[key].decimals == 3
111+
assert fields[key].minimum == 0.001
108112

109113
def test_proportional_zoom_is_a_checkbox(self):
110114
fields = {spec.settings_key: spec for spec in PLAYLIST_FIELDS}
111115

112-
assert fields["playlist/zoom_proportional"].kind is FieldKind.CHECKBOX
116+
assert isinstance(fields["playlist/zoom_proportional"], CheckboxField)
113117

114118

115119
class TestTheForm:

‎tests/test_subtitle_encoding.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ def test_it_sits_with_the_rest_of_the_subtitle_defaults(self):
212212
spec = next(f for f in VIDEO_FIELDS if f.settings_key == SETTING)
213213

214214
assert spec.video_attr == "subtitle_encoding"
215-
assert spec.combo_values is _subtitle_encodings
215+
assert spec.choices is _subtitle_encodings
216216
# it has to apply to a track picked from the menu too, not only to
217217
# one a video opens with, so nothing gates it
218218
assert spec.enabled_by is None

‎tests/test_sync.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
from gridplayer.models.grid_state import GridState
1616
from gridplayer.models.video import Video
1717
from gridplayer.params.actions import ACTIONS
18-
from gridplayer.params.defaults_fields import PLAYLIST_FIELDS, FieldKind
18+
from gridplayer.params.defaults_fields import PLAYLIST_FIELDS, CheckboxField
1919
from gridplayer.params.menu import SECTIONS, SUBMENUS
2020
from gridplayer.params.static import (
2121
SeekSyncMode,
@@ -181,7 +181,7 @@ def test_they_are_together_with_seek_sync_in_a_section_of_their_own(self):
181181
"pause_sync",
182182
"rate_sync",
183183
]
184-
assert all(field.kind == FieldKind.CHECKBOX for field in fields[1:])
184+
assert all(isinstance(field, CheckboxField) for field in fields[1:])
185185

186186

187187
def _block(is_initialized=True, has_picture=True, is_live=False):

‎tests/test_video_position.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
from gridplayer.models.playlist import Playlist, PlaylistVideoDefaults
2121
from gridplayer.models.video import Video
2222
from gridplayer.params.actions import ACTIONS
23-
from gridplayer.params.defaults_fields import VIDEO_FIELDS, FieldKind
23+
from gridplayer.params.defaults_fields import VIDEO_FIELDS, CheckboxField
2424
from gridplayer.params.menu import SECTIONS, SUBMENUS
2525
from gridplayer.params.static import (
2626
MAX_SCALE,
@@ -176,7 +176,7 @@ def test_the_alignment_sits_under_crop_in_the_video_section(self):
176176
spec = VIDEO_FIELDS[at + 1]
177177
assert spec.section == "Video"
178178
assert spec.video_attr == "anchor"
179-
assert list(spec.combo_values()) == list(VideoAnchor)
179+
assert list(spec.choices()) == list(VideoAnchor)
180180

181181
def test_letting_it_past_the_edges_sits_under_the_alignment(self):
182182
keys = [f.settings_key for f in VIDEO_FIELDS]
@@ -186,7 +186,7 @@ def test_letting_it_past_the_edges_sits_under_the_alignment(self):
186186

187187
spec = VIDEO_FIELDS[at + 1]
188188
assert spec.section == "Video"
189-
assert spec.kind == FieldKind.CHECKBOX
189+
assert isinstance(spec, CheckboxField)
190190
assert spec.video_attr == "shift_past_edges"
191191

192192
def test_zooming_to_the_cursor_sits_under_the_zoom(self):
@@ -197,7 +197,7 @@ def test_zooming_to_the_cursor_sits_under_the_zoom(self):
197197

198198
spec = VIDEO_FIELDS[at + 1]
199199
assert spec.section == "Video"
200-
assert spec.kind == FieldKind.CHECKBOX
200+
assert isinstance(spec, CheckboxField)
201201
assert spec.video_attr == "zoom_to_cursor"
202202

203203
def test_there_is_no_default_for_the_position(self):

‎tests/test_video_session_defaults.py‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@
66

77
from gridplayer.models.playlist import Playlist
88
from gridplayer.models.video import Video
9-
from gridplayer.params.defaults_fields import VIDEO_FIELDS
9+
from gridplayer.params.defaults_fields import (
10+
PLAYLIST_FIELDS,
11+
SUBTITLE_STYLE_FIELDS,
12+
VIDEO_FIELDS,
13+
)
1014
from gridplayer.params.static import VideoCrop, VideoEndAction, VideoInitialState
1115
from gridplayer.playlist_settings import (
1216
PlaylistSettings,
@@ -252,3 +256,19 @@ def test_settings_get_all_includes_new_defaults():
252256
assert values["video_defaults/rate"] == _default_settings["video_defaults/rate"]
253257
assert values["video_defaults/crop"] == VideoCrop(0, 0, 0, 0)
254258
assert values["video_defaults/color"] == "#ffffff"
259+
260+
261+
@pytest.mark.parametrize(
262+
"fields",
263+
[PLAYLIST_FIELDS, VIDEO_FIELDS, SUBTITLE_STYLE_FIELDS],
264+
ids=["playlist", "video", "subtitle_style"],
265+
)
266+
def test_defaults_form_gives_back_every_default_it_is_given(fields):
267+
defaults = {
268+
spec.settings_key: _default_settings[spec.settings_key] for spec in fields
269+
}
270+
form = DefaultsForm(fields, show_reset=True)
271+
272+
form.set_values(defaults)
273+
274+
assert form.values() == defaults

0 commit comments

Comments
 (0)