refactor: move the 89 off-grid spacing values onto the M3 scale
Phase 2, second step, of docs/material-design-conformance.md. 77 of the 89 literals that
were off M3's spacing scale sat in spacing positions and now read
`MaterialTheme.spacing.spaceNNN`; the remaining 12 are dimensions and are out of scope.
One drifted corner moved onto the shape scale.
**The mapping, and why each is the nearest stop rather than the nicest number.**
5.dp x10 -> space50 (4dp) padding and gaps in dense rows
15.dp x14 -> space200 (16dp) card and dialog padding, two gaps
30.dp x1 -> space400 (32dp) the spacer under LoadingDataIndicator's spinner
50.dp x52 -> space600 (48dp) the spacer above an empty or error message
Nearest-stop throughout, so the largest move is 2dp and most are 1. `5.dp` is equidistant
between `space50` and `space75`; it goes to 4dp because `spacedBy(4.dp)` is already the
idiom elsewhere in the tree and a scale with two answers for the same input is not one.
The 52 at 48dp are the same three lines copied into 16 files -- a `Spacer` pushing
"Something went wrong" down the screen. Phase 5 retires them into a shared empty-state
composable; migrating them first means that composable inherits a token rather than
another literal.
**One shape, and it is the argument for having a scale at all.**
`RoundedCornerShape(30.dp)` in `TextNoteEventDetail` was the only hand-written corner off
the M3 scale, at 30dp against `extraLarge`'s 28. Two units: invisible beside any single
other card, and exactly the drift that happens when the value is a literal. It is now
`MaterialTheme.shapes.extraLarge`, the first call site for the scale `Shape.kt` documented.
**Rewritten by a script that reads call shapes, not values, and it is checked in.**
`docs/scripts/m3-migrate-spacing.py` brace-matches three call shapes -- `padding(...)`/
`PaddingValues(...)`, `Arrangement.spacedBy(...)`, and a `.height()`/`.width()` whose
enclosing call is `Spacer(` -- and rewrites only literals that fall inside one. A
`.size(18.dp)` icon, a non-Spacer `.height()`, a `RoundedCornerShape` or a `BorderStroke`
can never be caught, which a regex over `\\d+\\.dp` would have done to all of them. It
inserts the two imports where they are missing and skips comment lines. Dry run by default.
**The audit was measuring the wrong thing, and this is where that showed.** It split
literals by value against a hardcoded `DIMENSION_EXEMPT` list -- and the split is not a
property of the value. `16.dp` is a spacing stop *and* a plausible icon size. `50.dp` was a
`Spacer` height in 52 places and a divider width in one, and no list of numbers separates
those. `docs/scripts/m3-spacing-positions.py` replaces it with the same brace-matching
parse the migration uses, so the audit and the migration agree by construction; the audit
now reports **353 spacing literals** left and 76 dimensions out of scope, and the exemption
table is gone.
That reframes phase 2's acceptance criterion into something checkable: spacing positions to
zero, dimensions untouched. The script exits 1 while any spacing literal remains.
**What is left off-scale, and why none of it is a defect.** Twelve dimensions: avatar sizes
at 35, 55, 70 and 75dp, icon sizes at 18 and 22dp, and a 50dp divider width. Avatar and
icon sizing is a component-spec question rather than a spacing one -- M3 gives icons 18/20/
24/40/48 and says nothing about avatars -- and the plan puts per-component specs after the
adaptive phase. They are reported rather than exempted so the number stays visible.
**Tests.** 942 pass, 594 jvm over 72 classes and 348 android over 44, unchanged --
this commit adds no assertions, and the ones it could add (`SpacingScaleTest`) landed with
the scale. `:composeApp:compileDebugKotlinAndroid` builds, `m3-audit.sh --check` exits 0.
Pixels move by at most 2dp, in 30 files.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -25,23 +25,13 @@ THEME="$UI/theme"
|
||||
# Budgets. "-1" means not yet budgeted -- reported, but never fails --check.
|
||||
# ---------------------------------------------------------------------------
|
||||
BUDGET_HARDCODED_COLOR=9 # phase 3 drives to 0 outside theme/
|
||||
BUDGET_DP_LITERALS=-1 # phase 2 drives to ~0 outside theme/
|
||||
BUDGET_OFF_SCALE_DP=-1 # phase 2 drives to 0
|
||||
BUDGET_SPACING_LITERALS=353 # phase 2 drives to 0
|
||||
BUDGET_BARE_CLICKABLE=33 # phase 3 drives to 0
|
||||
BUDGET_NULL_DESCRIPTION=18 # phase 3 triages each one
|
||||
BUDGET_STRING_LITERALS=-1 # phase 4 drives to <10
|
||||
BUDGET_TITLE_CASE=-1 # phase 4 drives to 0
|
||||
BUDGET_UNSET_COLOR_ROLES=0 # phase 1: reached 2026-09-07
|
||||
|
||||
# The M3 spacing scale: docs/material-design-conformance.md, "The numbers".
|
||||
# space0..space900. Anything outside this set is off-scale.
|
||||
ON_SCALE=(0 2 4 6 8 10 12 14 16 20 24 32 36 40 48 56 64 72)
|
||||
|
||||
# Dimensions rather than spacing -- an avatar, an image height, a hairline
|
||||
# border. These are exempt from the off-scale count; keep the list short and
|
||||
# justify additions in the commit that makes them.
|
||||
DIMENSION_EXEMPT=(1 80 128 180 200 500)
|
||||
|
||||
fail_count=0
|
||||
|
||||
hdr() { printf '\n\033[1m== %s\033[0m\n' "$1"; }
|
||||
@@ -136,40 +126,18 @@ report 'colours derived with .copy(alpha =)' "$alpha" -1
|
||||
# ---------------------------------------------------------------------------
|
||||
hdr 'Spacing (phase 2)'
|
||||
|
||||
dp_all=$(grep -rhoE '\b[0-9]+\.dp' "$UI" --include=*.kt 2>/dev/null \
|
||||
| grep -v "^$THEME/" | wc -l | tr -d ' ')
|
||||
dp_outside_theme=$(grep -rhoE '\b[0-9]+\.dp' \
|
||||
$(grep -rl '\.dp' "$UI" --include=*.kt 2>/dev/null | grep -v "^$THEME/") \
|
||||
2>/dev/null | wc -l | tr -d ' ')
|
||||
report '.dp literals outside theme/' "$dp_outside_theme" "$BUDGET_DP_LITERALS"
|
||||
# Classified by call shape rather than by value, which is the only thing that says
|
||||
# whether a given literal is spacing or a dimension: 16.dp is a spacing stop and also a
|
||||
# plausible icon size, and 50.dp was a Spacer height in 53 places and a divider width in
|
||||
# one. m3-spacing-positions.py does the parse; it exits 1 while any spacing literal is
|
||||
# left, which is phase 2's acceptance criterion.
|
||||
spacing_report=$(python3 docs/scripts/m3-spacing-positions.py)
|
||||
spacing_left=$(echo "$spacing_report" | awk '/spacing positions/ {print $NF}')
|
||||
dimensions=$(echo "$spacing_report" | grep 'dimension positions' | grep -oE '[0-9]+')
|
||||
report 'dp literals in spacing positions' "$spacing_left" "$BUDGET_SPACING_LITERALS"
|
||||
note "in dimension positions (out of scope): $dimensions"
|
||||
note 'run docs/scripts/m3-spacing-positions.py --list to see them'
|
||||
|
||||
# Split the histogram into on-scale, exempt dimensions, and off-scale.
|
||||
declare -A hist
|
||||
while read -r n; do
|
||||
hist[$n]=$(( ${hist[$n]:-0} + 1 ))
|
||||
done < <(grep -rhoE '\b[0-9]+\.dp' \
|
||||
$(grep -rl '\.dp' "$UI" --include=*.kt 2>/dev/null | grep -v "^$THEME/") \
|
||||
2>/dev/null | sed 's/\.dp//')
|
||||
|
||||
on_scale_total=0; off_scale_total=0; exempt_total=0; off_scale_detail=""
|
||||
for n in "${!hist[@]}"; do
|
||||
c=${hist[$n]}
|
||||
if printf '%s\n' "${ON_SCALE[@]}" | grep -qx "$n"; then
|
||||
on_scale_total=$((on_scale_total + c))
|
||||
elif printf '%s\n' "${DIMENSION_EXEMPT[@]}" | grep -qx "$n"; then
|
||||
exempt_total=$((exempt_total + c))
|
||||
else
|
||||
off_scale_total=$((off_scale_total + c))
|
||||
off_scale_detail="$off_scale_detail ${n}dp:${c}"
|
||||
fi
|
||||
done
|
||||
note "on the M3 scale: $on_scale_total"
|
||||
note "exempt dimensions: $exempt_total"
|
||||
report 'off the M3 spacing scale' "$off_scale_total" "$BUDGET_OFF_SCALE_DP"
|
||||
[[ -n $off_scale_detail ]] && note "off-scale:$off_scale_detail"
|
||||
|
||||
spacer_idiom=$(count 'height\(50\.dp\)')
|
||||
note "Spacer(height(50.dp)) idiom: $spacer_idiom"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
hdr 'Typography (phase 1)'
|
||||
|
||||
149
docs/scripts/m3-migrate-spacing.py
Executable file
149
docs/scripts/m3-migrate-spacing.py
Executable file
@@ -0,0 +1,149 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Rewrite .dp literals that sit in spacing positions onto MaterialTheme.spacing.
|
||||
|
||||
Checked in because phase 2 runs it twice and the breakpoint phase will want it again.
|
||||
Usage:
|
||||
|
||||
m3-migrate-spacing.py '{"5":"space50","15":"space200"}' # dry run
|
||||
m3-migrate-spacing.py '{"5":"space50","15":"space200"}' --apply
|
||||
|
||||
Only three call shapes are touched, and each is matched with the literal in place so a
|
||||
dimension can never be caught by accident:
|
||||
|
||||
padding(...) any of the overloads, including named start/end/top/bottom
|
||||
Arrangement.spacedBy(N) horizontal or vertical
|
||||
Spacer height/width a .height()/.width() whose enclosing call is Spacer(
|
||||
|
||||
Everything else -- .size(), a non-Spacer .height(), RoundedCornerShape, BorderStroke --
|
||||
is a dimension and is left alone.
|
||||
"""
|
||||
import re, sys, io, os
|
||||
|
||||
UI = 'composeApp/src/commonMain/kotlin/press/mantra/compose/ui'
|
||||
MAPPING = {} # filled by the caller
|
||||
DRY = '--apply' not in sys.argv
|
||||
|
||||
def token(value):
|
||||
return MAPPING.get(value)
|
||||
|
||||
def spacer_spans(text):
|
||||
"""Character ranges covered by a Spacer( ... ) call, brace-matched."""
|
||||
spans = []
|
||||
for m in re.finditer(r'\bSpacer\s*\(', text):
|
||||
depth, i = 0, m.end() - 1
|
||||
while i < len(text):
|
||||
if text[i] == '(':
|
||||
depth += 1
|
||||
elif text[i] == ')':
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
spans.append((m.start(), i))
|
||||
break
|
||||
i += 1
|
||||
return spans
|
||||
|
||||
def in_spans(pos, spans):
|
||||
return any(a <= pos <= b for a, b in spans)
|
||||
|
||||
def padding_spans(text):
|
||||
spans = []
|
||||
for m in re.finditer(r'\.?\bpadding\s*\(|\bPaddingValues\s*\(', text):
|
||||
depth, i = 0, m.end() - 1
|
||||
while i < len(text):
|
||||
if text[i] == '(':
|
||||
depth += 1
|
||||
elif text[i] == ')':
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
spans.append((m.start(), i))
|
||||
break
|
||||
i += 1
|
||||
return spans
|
||||
|
||||
def spacedby_spans(text):
|
||||
spans = []
|
||||
for m in re.finditer(r'\bspacedBy\s*\(', text):
|
||||
depth, i = 0, m.end() - 1
|
||||
while i < len(text):
|
||||
if text[i] == '(':
|
||||
depth += 1
|
||||
elif text[i] == ')':
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
spans.append((m.start(), i))
|
||||
break
|
||||
i += 1
|
||||
return spans
|
||||
|
||||
def process(path):
|
||||
text = io.open(path, encoding='utf-8').read()
|
||||
original = text
|
||||
changed = []
|
||||
|
||||
for _ in range(60): # spans shift after each edit; recompute
|
||||
pads = padding_spans(text)
|
||||
gaps = spacedby_spans(text)
|
||||
spacers = spacer_spans(text)
|
||||
# a .height()/.width() literal counts only inside a Spacer(
|
||||
hw = [(m.start(1), m.end(1), m.group(1))
|
||||
for m in re.finditer(r'\.(?:height|width)\s*\(\s*(\d+\.dp)\s*\)', text)
|
||||
if in_spans(m.start(), spacers)]
|
||||
|
||||
edit = None
|
||||
for m in re.finditer(r'\b(\d+)\.dp\b', text):
|
||||
lit = m.group(0)
|
||||
tok = token(m.group(1))
|
||||
if tok is None:
|
||||
continue
|
||||
pos = m.start()
|
||||
line_start = text.rfind('\n', 0, pos) + 1
|
||||
line = text[line_start:text.find('\n', pos)]
|
||||
if re.match(r'\s*(//|\*|/\*)', line): # a comment
|
||||
continue
|
||||
if in_spans(pos, pads) or in_spans(pos, gaps) or \
|
||||
any(a <= pos < b for a, b, _ in hw):
|
||||
edit = (m.start(), m.end(), tok, lit, line.strip()[:80])
|
||||
break
|
||||
if edit is None:
|
||||
break
|
||||
a, b, tok, lit, ctx = edit
|
||||
text = text[:a] + f'MaterialTheme.spacing.{tok}' + text[b:]
|
||||
changed.append((lit, tok, ctx))
|
||||
|
||||
if text != original:
|
||||
if 'import androidx.compose.material3.MaterialTheme' not in text:
|
||||
# insert alphabetically among the material3 imports, else after the last import
|
||||
lines = text.split('\n')
|
||||
idx = max(i for i, l in enumerate(lines) if l.startswith('import '))
|
||||
for i, l in enumerate(lines):
|
||||
if l.startswith('import androidx.compose.material3.') and l > 'import androidx.compose.material3.MaterialTheme':
|
||||
idx = i - 1
|
||||
break
|
||||
lines.insert(idx + 1, 'import androidx.compose.material3.MaterialTheme')
|
||||
text = '\n'.join(lines)
|
||||
if 'import press.mantra.compose.ui.theme.spacing' not in text and \
|
||||
not path.endswith('theme/Spacing.kt'):
|
||||
lines = text.split('\n')
|
||||
idx = max(i for i, l in enumerate(lines) if l.startswith('import '))
|
||||
lines.insert(idx + 1, 'import press.mantra.compose.ui.theme.spacing')
|
||||
text = '\n'.join(lines)
|
||||
if not DRY:
|
||||
io.open(path, 'w', encoding='utf-8').write(text)
|
||||
return changed
|
||||
|
||||
if __name__ == '__main__':
|
||||
import json
|
||||
MAPPING.update(json.loads(sys.argv[1]))
|
||||
total = 0
|
||||
for root, _, files in os.walk(UI):
|
||||
for f in sorted(files):
|
||||
if not f.endswith('.kt'):
|
||||
continue
|
||||
p = os.path.join(root, f)
|
||||
ch = process(p)
|
||||
if ch:
|
||||
print(f'{p.replace(UI + "/", "")}')
|
||||
for lit, tok, ctx in ch:
|
||||
print(f' {lit:>7s} -> {tok:18s} {ctx}')
|
||||
total += len(ch)
|
||||
print(f'\n{total} literal(s) {"would be " if DRY else ""}rewritten')
|
||||
94
docs/scripts/m3-spacing-positions.py
Executable file
94
docs/scripts/m3-spacing-positions.py
Executable file
@@ -0,0 +1,94 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Count .dp literals by whether they sit in a spacing position or a dimension one.
|
||||
|
||||
m3-audit.sh used to split them by value, exempting a list of numbers that "looked like"
|
||||
dimensions -- which is wrong twice over. 16.dp is a spacing stop *and* a plausible icon
|
||||
size, and 50.dp was a Spacer height in 53 places and a divider width in one. Only the call
|
||||
shape says which, so this reads the shape.
|
||||
|
||||
spacing padding(...), PaddingValues(...), Arrangement.spacedBy(...), and a
|
||||
.height()/.width() whose enclosing call is Spacer(
|
||||
dimension .size(), a non-Spacer .height()/.width(), RoundedCornerShape(),
|
||||
BorderStroke(), a `size =` argument
|
||||
|
||||
Spacing positions are what phase 2 drives to zero; dimensions are out of its scope and are
|
||||
reported so the number is visible rather than exempted.
|
||||
|
||||
Usage: m3-spacing-positions.py [--list]
|
||||
"""
|
||||
import os, re, sys
|
||||
|
||||
UI = 'composeApp/src/commonMain/kotlin/press/mantra/compose/ui'
|
||||
THEME = os.path.join(UI, 'theme')
|
||||
|
||||
|
||||
def _spans(text, opener):
|
||||
"""Brace-matched character ranges of every call matching `opener`."""
|
||||
spans = []
|
||||
for m in re.finditer(opener, text):
|
||||
depth, i = 0, m.end() - 1
|
||||
while i < len(text):
|
||||
if text[i] == '(':
|
||||
depth += 1
|
||||
elif text[i] == ')':
|
||||
depth -= 1
|
||||
if depth == 0:
|
||||
spans.append((m.start(), i))
|
||||
break
|
||||
i += 1
|
||||
return spans
|
||||
|
||||
|
||||
def classify(path):
|
||||
text = open(path, encoding='utf-8').read()
|
||||
pads = _spans(text, r'\.?\bpadding\s*\(|\bPaddingValues\s*\(')
|
||||
gaps = _spans(text, r'\bspacedBy\s*\(')
|
||||
spacers = _spans(text, r'\bSpacer\s*\(')
|
||||
|
||||
def inside(pos, spans):
|
||||
return any(a <= pos <= b for a, b in spans)
|
||||
|
||||
hw = [
|
||||
(m.start(1), m.end(1))
|
||||
for m in re.finditer(r'\.(?:height|width)\s*\(\s*(\d+\.dp)\s*\)', text)
|
||||
if inside(m.start(), spacers)
|
||||
]
|
||||
|
||||
spacing, dimension = [], []
|
||||
for m in re.finditer(r'\b(\d+)\.dp\b', text):
|
||||
pos = m.start()
|
||||
line_start = text.rfind('\n', 0, pos) + 1
|
||||
line = text[line_start:text.find('\n', pos)]
|
||||
if re.match(r'\s*(//|\*|/\*)', line):
|
||||
continue
|
||||
entry = (path, text[:pos].count('\n') + 1, m.group(0), line.strip()[:90])
|
||||
if inside(pos, pads) or inside(pos, gaps) or any(a <= pos < b for a, b in hw):
|
||||
spacing.append(entry)
|
||||
else:
|
||||
dimension.append(entry)
|
||||
return spacing, dimension
|
||||
|
||||
|
||||
def main():
|
||||
spacing, dimension = [], []
|
||||
for root, _, files in os.walk(UI):
|
||||
if root.startswith(THEME):
|
||||
continue
|
||||
for f in sorted(files):
|
||||
if f.endswith('.kt'):
|
||||
s, d = classify(os.path.join(root, f))
|
||||
spacing += s
|
||||
dimension += d
|
||||
|
||||
print(f' {"dp literals in spacing positions":42s} {len(spacing):6d}')
|
||||
print(f' {"dp literals in dimension positions":42s} {len(dimension):6d} (out of scope)')
|
||||
|
||||
if '--list' in sys.argv:
|
||||
for path, line, lit, ctx in spacing:
|
||||
print(f' {path.replace(UI + "/", "")}:{line} {lit:>7s} {ctx}')
|
||||
|
||||
return 1 if spacing else 0
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
sys.exit(main())
|
||||
Reference in New Issue
Block a user