Skip to content

Consolidate aesthetic information#135

Merged
thomasp85 merged 2 commits intoposit-dev:mainfrom
thomasp85:aesthetics
Feb 18, 2026
Merged

Consolidate aesthetic information#135
thomasp85 merged 2 commits intoposit-dev:mainfrom
thomasp85:aesthetics

Conversation

@thomasp85
Copy link
Copy Markdown
Collaborator

This PR adds an aesthetic module which takes on the responsibility of handling aesthetic names etc. It creates a single source of truth throughout the code base and importantly remove a whole range of hardcoded matching for positional aesthetics

@thomasp85 thomasp85 requested a review from teunbrand February 18, 2026 10:29
Copy link
Copy Markdown
Collaborator

@teunbrand teunbrand left a comment

Choose a reason for hiding this comment

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

A few nitpicks but this LGTM.
Something beyond this PR is whether we should automate writing the aesthetic_name grammar component based on these definitions. See

aesthetic_name: $ => choice(
// Position aesthetics
'x', 'y', 'xmin', 'xmax', 'ymin', 'ymax', 'xend', 'yend',
// Aggregation aesthetic (for bar charts)
'weight',
// Color aesthetics
'color', 'colour', 'fill', 'stroke', 'opacity',
// Size and shape
'size', 'shape', 'linetype', 'linewidth', 'width', 'height',
// Text aesthetics
'label', 'family', 'fontface', 'hjust', 'vjust'
),
.

Comment thread src/plot/aesthetic.rs Outdated
Comment thread src/plot/layer/geom/types.rs Outdated
Comment thread src/plot/aesthetic.rs Outdated
Comment thread src/plot/aesthetic.rs
@teunbrand
Copy link
Copy Markdown
Collaborator

teunbrand commented Feb 18, 2026

Another thought that occurs to me: what if this was implemented as a struct? Like one Aesthetics struct containing different fields like positional with aesthetic names and methods like is_primary_position() etc. We'd don't have to faff with importing the precise checkers and whatnot because they'd be encapsulated. I'm not saying this is necessarily a better design decision, but might be worth giving a small think.

@thomasp85 thomasp85 merged commit 559dec8 into posit-dev:main Feb 18, 2026
4 checks passed
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