Skip to content

Refactor of layout utils#6442

Merged
snejus merged 8 commits intomasterfrom
refactor-layout-utils
Mar 17, 2026
Merged

Refactor of layout utils#6442
snejus merged 8 commits intomasterfrom
refactor-layout-utils

Conversation

@snejus
Copy link
Member

@snejus snejus commented Mar 14, 2026

Summary

This PR refactors import-match layout rendering by centralizing layout selection and line generation in beets.util.layout, simplifying the ShowChange display path, and tightening the layout
data model.

What Changed

  • Added get_layout_lines() and get_layout_method() in beets.util.layout so layout selection (column vs newline) is handled in one place.
  • Replaced Side from a mutable TypedDict with an immutable NamedTuple that exposes derived helpers (rendered, prefix/suffix widths, and rendered width).
  • Simplified split_into_lines() from a 3-width tuple API to (first_width, width) and removed legacy last-line empty-string handling.
  • Refactored beets.ui.commands.import_/display.py ShowChange to call get_layout_lines() directly and removed duplicate per-class layout-selection logic.
  • Updated tracklist width calculation to use Side helpers and explicit width assignment via _replace(width=...).
  • Reworked ShowChange tests into snapshot-style assertions for both newline and column layouts, and updated util layout tests to the new split_into_lines() signature.

Why

  • Reduces duplicated wrapping/layout logic across UI code paths.
  • Makes layout behavior easier to reason about and test at the utility boundary.
  • Narrows the display layer to orchestration while keeping transformation/rendering logic in reusable utilities.
  • Improves maintainability by moving from loosely typed dict mutation to a typed, self-describing data structure.

@snejus snejus requested a review from JOJ0 March 14, 2026 19:29
@snejus snejus requested a review from a team as a code owner March 14, 2026 19:29
@snejus snejus requested a review from Serene-Arc March 14, 2026 19:29
@github-actions
Copy link

Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry.

@snejus
Copy link
Member Author

snejus commented Mar 14, 2026

@JOJ0 this one for you to have a look at!

@codecov
Copy link

codecov bot commented Mar 14, 2026

Codecov Report

❌ Patch coverage is 91.34615% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.73%. Comparing base (05f0ec3) to head (cfc7ffb).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
beets/util/layout.py 89.65% 6 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6442      +/-   ##
==========================================
+ Coverage   69.69%   69.73%   +0.03%     
==========================================
  Files         145      145              
  Lines       18541    18534       -7     
  Branches     3028     3023       -5     
==========================================
+ Hits        12923    12924       +1     
+ Misses       4982     4976       -6     
+ Partials      636      634       -2     
Files with missing lines Coverage Δ
beets/ui/commands/import_/display.py 82.18% <100.00%> (+0.26%) ⬆️
beets/util/layout.py 83.55% <89.65%> (+4.10%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@snejus snejus force-pushed the refactor-layout-utils branch 3 times, most recently from 7bda104 to fb3ff75 Compare March 15, 2026 08:17
@snejus snejus changed the title Small refactor of layout utils Refactor of layout utils Mar 15, 2026
@snejus snejus requested a review from Copilot March 15, 2026 09:13
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors import-match layout rendering by replacing TypedDict Side with an immutable NamedTuple, simplifying split_into_lines from a 3-width tuple to two args, and centralizing layout selection logic in beets.util.layout.

Changes:

  • Converted Side from TypedDict to NamedTuple with derived helper properties (rendered, prefix_width, etc.) and simplified split_into_lines signature.
  • Added get_layout_lines() and cached get_layout_method() in beets.util.layout, removing duplicate layout-selection logic from ShowChange.
  • Rewrote ShowChange tests as snapshot-style assertions for both layout modes.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
beets/util/layout.py Core refactor: SideNamedTuple, simplified split_into_lines, added get_layout_lines/get_layout_method
beets/ui/commands/import_/display.py Simplified to use Side constructors and get_layout_lines directly
test/util/test_layout.py Updated split_into_lines calls to new 2-arg signature
test/ui/commands/test_import.py Replaced granular tests with snapshot assertions for both layouts

You can also share your feedback on Copilot code review. Take the survey.

@snejus snejus force-pushed the refactor-layout-utils branch from 4d44c33 to d65f70f Compare March 15, 2026 09:25
Base automatically changed from move-utils-from-ui-to-util to master March 16, 2026 01:28
@snejus snejus force-pushed the refactor-layout-utils branch from d65f70f to 34288b9 Compare March 16, 2026 01:30
@snejus
Copy link
Member Author

snejus commented Mar 16, 2026

@JOJ0 added a commit to document Side.

@snejus snejus force-pushed the refactor-layout-utils branch 2 times, most recently from c5f1135 to cfc7ffb Compare March 17, 2026 08:20
@snejus snejus force-pushed the refactor-layout-utils branch from cfc7ffb to a3e94ec Compare March 17, 2026 18:00
@snejus snejus merged commit 1943b14 into master Mar 17, 2026
18 checks passed
@snejus snejus deleted the refactor-layout-utils branch March 17, 2026 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants