feat: add preferences.density preset to scale spacing tokens uniformly#127
Open
smur89 wants to merge 1 commit into
Open
feat: add preferences.density preset to scale spacing tokens uniformly#127smur89 wants to merge 1 commit into
smur89 wants to merge 1 commit into
Conversation
Adds `preferences.density` — a single coherent knob that scales every vertical spacing em-token uniformly, replacing the need for users to hand-tune `bodySize`, `margin`, and individual em-multipliers when they want a tighter (one-page fit) or roomier (print presentation) layout. Three presets: `"compact"` (× 0.85), `"comfortable"` (× 1.0, default), `"spacious"` (× 1.15). Closes #50. The multiplier is threaded through a new `_spacing_scale_state` and applied at every block `above`/`below`, every `v()`, `par.spacing` / `leading`, and `list.spacing`. Text sizes, icon dimensions, and rating-dot geometry are intentionally left alone — density is purely about vertical whitespace, so font-size scaling stays under `bodySize`. Default ("comfortable") is render-identical to a no-density build — verified by re-running `examples/tests/*.pdf` against the tree before and after the change; all byte sizes unchanged. Also fixes a latent issue in `sections/projects.typ` where the description → term gap used a literal `0.6em` and so didn't scale with density, breaking the "matches institution-line → term spacing" invariant the comment claimed. Now uses `0.6 * scale * body-size` to mirror `name()`'s `below`.
5 tasks
📝 WalkthroughWalkthroughAdds a ChangesDensity Preset Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/density.typ`:
- Around line 10-50: The test CV data in the cv variable is missing a projects
section, which means the density spacing fix in sections/projects.typ (where
hardcoded 0.6em spacing now scales with density) is not being exercised by the
regression test. Add a projects section to the cv data structure with
appropriate project entries (name, description, startDate, endDate, etc.) to
ensure the spacing behavior is tested and any future regressions in project
description spacing are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: aa783a29-c83f-4b46-babe-ea3dd88da50e
⛔ Files ignored due to path filters (1)
examples/tests/density.pdfis excluded by!**/*.pdf
📒 Files selected for processing (9)
README.mdinternal/header.typinternal/layout.typinternal/primitives.typinternal/state.typlib.typsections/projects.typsections/skills.typtests/density.typ
Comment on lines
+10
to
+50
| #let cv = ( | ||
| basics: ( | ||
| name: "Jane Doe", | ||
| label: "Senior Software Engineer", | ||
| summary: [Backend engineer; the summary block exercises the | ||
| pre/post `v()` gaps that flank it.], | ||
| email: "jane@example.com", | ||
| phone: "+353 1 555 0100", | ||
| location: "Dublin, Ireland", | ||
| ), | ||
| work: ( | ||
| ( | ||
| name: "Acme Corp", | ||
| position: "Senior Software Engineer", | ||
| location: "Dublin, Ireland", | ||
| startDate: "Jan 2022", | ||
| highlights: ([Led the migration.], [Designed the platform.]), | ||
| ), | ||
| ( | ||
| name: "Foo Ltd", | ||
| position: "Software Engineer", | ||
| startDate: "Jan 2018", | ||
| endDate: "Dec 2021", | ||
| highlights: ([Shipped the thing.],), | ||
| ), | ||
| ), | ||
| skills: ( | ||
| (name: "Languages", keywords: ("Scala", "Python")), | ||
| (name: "Infra", keywords: ("Kafka", "AWS", "Kubernetes")), | ||
| ), | ||
| languages: ( | ||
| (language: "English", fluency: "Native"), | ||
| (language: "Irish", fluency: "Professional Working"), | ||
| ), | ||
| education: ( | ||
| (institution: "Example U", studyType: "B.Sc.", endDate: "2017"), | ||
| ), | ||
| certificates: ( | ||
| (name: "CKA", issuer: "CNCF"), | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Consider adding a projects section to cover the sections/projects.typ spacing fix.
The PR objectives mention a latent bug fix in sections/projects.typ where hardcoded 0.6em spacing now scales with density. The test CV data doesn't include a projects section, so the regression test won't exercise that fix or catch future regressions in project description spacing.
📋 Suggested addition to CV data
education: (
(institution: "Example U", studyType: "B.Sc.", endDate: "2017"),
),
+ projects: (
+ (
+ name: "Open Source Tool",
+ description: [Built a CLI tool; exercises the project description spacing that was fixed.],
+ highlights: ([Released v1.0.],),
+ ),
+ ),
certificates: (
(name: "CKA", issuer: "CNCF"),
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #let cv = ( | |
| basics: ( | |
| name: "Jane Doe", | |
| label: "Senior Software Engineer", | |
| summary: [Backend engineer; the summary block exercises the | |
| pre/post `v()` gaps that flank it.], | |
| email: "jane@example.com", | |
| phone: "+353 1 555 0100", | |
| location: "Dublin, Ireland", | |
| ), | |
| work: ( | |
| ( | |
| name: "Acme Corp", | |
| position: "Senior Software Engineer", | |
| location: "Dublin, Ireland", | |
| startDate: "Jan 2022", | |
| highlights: ([Led the migration.], [Designed the platform.]), | |
| ), | |
| ( | |
| name: "Foo Ltd", | |
| position: "Software Engineer", | |
| startDate: "Jan 2018", | |
| endDate: "Dec 2021", | |
| highlights: ([Shipped the thing.],), | |
| ), | |
| ), | |
| skills: ( | |
| (name: "Languages", keywords: ("Scala", "Python")), | |
| (name: "Infra", keywords: ("Kafka", "AWS", "Kubernetes")), | |
| ), | |
| languages: ( | |
| (language: "English", fluency: "Native"), | |
| (language: "Irish", fluency: "Professional Working"), | |
| ), | |
| education: ( | |
| (institution: "Example U", studyType: "B.Sc.", endDate: "2017"), | |
| ), | |
| certificates: ( | |
| (name: "CKA", issuer: "CNCF"), | |
| ), | |
| ) | |
| `#let` cv = ( | |
| basics: ( | |
| name: "Jane Doe", | |
| label: "Senior Software Engineer", | |
| summary: [Backend engineer; the summary block exercises the | |
| pre/post `v()` gaps that flank it.], | |
| email: "jane@example.com", | |
| phone: "+353 1 555 0100", | |
| location: "Dublin, Ireland", | |
| ), | |
| work: ( | |
| ( | |
| name: "Acme Corp", | |
| position: "Senior Software Engineer", | |
| location: "Dublin, Ireland", | |
| startDate: "Jan 2022", | |
| highlights: ([Led the migration.], [Designed the platform.]), | |
| ), | |
| ( | |
| name: "Foo Ltd", | |
| position: "Software Engineer", | |
| startDate: "Jan 2018", | |
| endDate: "Dec 2021", | |
| highlights: ([Shipped the thing.],), | |
| ), | |
| ), | |
| skills: ( | |
| (name: "Languages", keywords: ("Scala", "Python")), | |
| (name: "Infra", keywords: ("Kafka", "AWS", "Kubernetes")), | |
| ), | |
| languages: ( | |
| (language: "English", fluency: "Native"), | |
| (language: "Irish", fluency: "Professional Working"), | |
| ), | |
| education: ( | |
| (institution: "Example U", studyType: "B.Sc.", endDate: "2017"), | |
| ), | |
| projects: ( | |
| ( | |
| name: "Open Source Tool", | |
| description: [Built a CLI tool; exercises the project description spacing that was fixed.], | |
| highlights: ([Released v1.0.],), | |
| ), | |
| ), | |
| certificates: ( | |
| (name: "CKA", issuer: "CNCF"), | |
| ), | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/density.typ` around lines 10 - 50, The test CV data in the cv variable
is missing a projects section, which means the density spacing fix in
sections/projects.typ (where hardcoded 0.6em spacing now scales with density) is
not being exercised by the regression test. Add a projects section to the cv
data structure with appropriate project entries (name, description, startDate,
endDate, etc.) to ensure the spacing behavior is tested and any future
regressions in project description spacing are caught.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
preferences.density— a single coherent knob that scales every vertical spacing em-token uniformly. Three presets:"compact"(× 0.85),"comfortable"(× 1.0, default),"spacious"(× 1.15).Closes #50. Supersedes #87 (predates the
lib.typ→internal//sections/split,internal/labels-en.tomlextraction, andexample_full.typdemo).Design
The multiplier is threaded through a new
_spacing_scale_state(alongside the existing_body_size_state/_accent_state) and applied at every blockabove/below, everyv(),par.spacing/leading, andlist.spacing. Text sizes, icon dimensions, and rating-dot geometry are intentionally left alone — density is purely vertical whitespace; font-size scaling stays underbodySize."comfortable"(default) is render-identical to a no-density build — verified by regenerating everyexamples/tests/*.pdfagainst the tree before and after the change; all byte sizes unchanged.Latent fix in
sections/projects.typThe /review pass surfaced an unscaled
block(below: 0.6em, ...)on the projects description line. The accompanying comment said the gap "matches the institution-line → term spacing in_experience/_education" — which routes throughname(), which IS scaled. So the literal0.6emwould have broken that contract under non-default density. Now uses0.6 * scale * body-sizeto mirrorname()'sbelow. Fixture-byte-identical at density 1.0; only differs at compact/spacious.Test plan
make test— every fixture compiles green, including the newtests/density.typ(three pages: compact / comfortable / spacious).examples/example_full.typ, all tracked PDFs unchanged).Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests