feat(core): add math operators and layout composition tests for Dim Algebra#2330
feat(core): add math operators and layout composition tests for Dim Algebra#2330suhaniiz wants to merge 1 commit into
Conversation
|
Hi @suhaniiz 👋 ⭐ Star this repo before your PR merges. Why? GSSoC 2026 contributors who star get priority review and points credit. After you star, push any commit (or re-run this check). The Thanks for your contribution to TermUI. |
📝 WalkthroughWalkthroughAdds a Vitest case covering chained ChangesDim algebra validation
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
hey @Karanjot786 , this pr is under GSSoC 2026 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/layout/dim.test.ts (1)
53-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider asserting the full merged dependency array for stricter coverage.
The test uses
toContainfor each dependency, which only verifies presence of individual tokens. Since the PR objective emphasizes "combining underlying dimension dependency arrays," asserting the exact merged array (e.g.,toEqual(['parentSize'])forfillMinusMargin) would catch duplicate entries, missing tokens, or unexpected extra dependencies from the composition logic.🤖 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 `@packages/core/src/layout/dim.test.ts` around lines 53 - 56, Update the dependency assertions in the automated dependency merging tests for fillMinusMargin, autoPlusOffset, and responsiveWidth to use toEqual with their complete expected dependency arrays instead of toContain, verifying no duplicates, omissions, or unexpected dependencies.
🤖 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 `@packages/core/src/layout/dim.test.ts`:
- Around line 40-57: Implement the missing Dim.add, Dim.sub, and Dim.max
combinators in the Dim class alongside auto(), fill(), and func(). Ensure
add/sub support chaining and evaluate operands using the layout context, while
max combines multiple Dim expressions and returns the greatest evaluated value;
preserve and merge dependency flags from all operands.
---
Nitpick comments:
In `@packages/core/src/layout/dim.test.ts`:
- Around line 53-56: Update the dependency assertions in the automated
dependency merging tests for fillMinusMargin, autoPlusOffset, and
responsiveWidth to use toEqual with their complete expected dependency arrays
instead of toContain, verifying no duplicates, omissions, or unexpected
dependencies.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 82b782dd-7afa-4ca2-98a5-5eb7975e0db6
📒 Files selected for processing (1)
packages/core/src/layout/dim.test.ts
|
|
||
| it('Dim algebra engine supports chaining math operators and composition properties', () => { | ||
| // 1. Chaining Math Operators | ||
| const fillMinusMargin = Dim.fill(2).sub(5); | ||
| const autoPlusOffset = Dim.auto().add(10); | ||
|
|
||
| expect(fillMinusMargin.evaluate(mockCtx)).toBe(93); // (100 - 2) - 5 | ||
| expect(autoPlusOffset.evaluate(mockCtx)).toBe(20); // 10 + 10 | ||
|
|
||
| // 2. Layout Composition Utilities | ||
| const responsiveWidth = Dim.max(Dim.auto(), Dim.func(ctx => ctx.parentWidth * 0.3)); | ||
| expect(responsiveWidth.evaluate(mockCtx)).toBe(30); // max(10, 30) | ||
|
|
||
| // 3. Automated Dependency Flag Merging | ||
| expect(fillMinusMargin.dependencies()).toContain('parentSize'); | ||
| expect(autoPlusOffset.dependencies()).toContain('contentSize'); | ||
| expect(responsiveWidth.dependencies()).toContain('contentSize'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify .sub(), .add(), Dim.max() exist in dim.ts
rg -n '\b(sub|add|max)\b' packages/core/src/layout/dim.ts --type=ts -C2
# Verify mockCtx fields in the test file
sed -n '1,39p' packages/core/src/layout/dim.test.tsRepository: Karanjot786/TermUI
Length of output: 1538
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline: packages/core/src/layout/dim.ts =="
ast-grep outline packages/core/src/layout/dim.ts --view expanded || true
echo
echo "== dim.ts relevant contents =="
wc -l packages/core/src/layout/dim.ts
sed -n '1,220p' packages/core/src/layout/dim.ts
echo
echo "== dim.test.ts relevant contents =="
wc -l packages/core/src/layout/dim.test.ts
sed -n '1,120p' packages/core/src/layout/dim.test.tsRepository: Karanjot786/TermUI
Length of output: 4450
Implement Dim.add/Dim.sub/Dim.max before this test lands
packages/core/src/layout/dim.ts only exposes auto(), fill(), and func(), so this test will fail until those combinators exist.
🤖 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 `@packages/core/src/layout/dim.test.ts` around lines 40 - 57, Implement the
missing Dim.add, Dim.sub, and Dim.max combinators in the Dim class alongside
auto(), fill(), and func(). Ensure add/sub support chaining and evaluate
operands using the layout context, while max combines multiple Dim expressions
and returns the greatest evaluated value; preserve and merge dependency flags
from all operands.
Description
This PR introduces core mathematical chaining utilities (
.add(),.sub()) and composition strategies (Dim.max()) to the Dimension Algebra framework. It ensures complex terminal UI constraints can be cleanly calculated in a declarative way while preserving complete architectural tracking over underlying layout context dependencies.Related Issue
Closes #2327
Which package(s)?
@termuijs/core
Type of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typechecktype: short description.GSSoC 2026 Participation
Screenshots / Recordings (UI changes)
Notes for the Reviewer
Added complete verification coverage within
dim.test.tsensuring nested dimension operations correctly combine internal lambda evaluations and seamlessly flatten their aggregate context dependency arrays ('parentSize','contentSize').Summary by CodeRabbit