Refactor length parsing and make offset a length#1330
Refactor length parsing and make offset a length#1330bynect merged 5 commits intodunst-project:masterfrom
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1330 +/- ##
==========================================
+ Coverage 65.71% 65.95% +0.23%
==========================================
Files 50 50
Lines 8041 8209 +168
Branches 925 962 +37
==========================================
+ Hits 5284 5414 +130
- Misses 2757 2795 +38
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Looks fine to me. Don't have time to review the whole thing though |
|
Can I go ahead? |
|
I haven't looked into the code in detail yet, but treating offset as a length of (min, max) seems odd as the values stored therein have completely different meaning, though they have the same structure. What is puzzling me is that Maybe I am missing the bigger picture here. |
My idea was making in a future refactor of the parser a unified "tuple"-like type. Also it is odd for me that we have the syntax for couple of values but offset is the only one using 1x2. For the height I wanted to add a min and max because dynamic heights was one of the low hanging fruit I am gonna add soon (aka one of the next prs) |
Move sanitization from the length parser to the settings file. Also make offset a length with the (N,N) syntax.
offset still accepts the old syntax for backward compatibility.
This will lay the foundations for implementing #991 and dynamic height.
This is a partial reversal of commit acf0a0f, @fwsmit what do you think?