Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(grit): add formatting of more nodes #4472

Merged
merged 1 commit into from
Nov 5, 2024
Merged

Conversation

ematipico
Copy link
Member

Summary

This PR adds more formatting for grit nodes. It also adds implement the debugging function that we will use in the playground. I will follow up with a PR in the website repository.

@arendjr I noticed we don't have parsing tests for many nodes/cases. I added few of them, but maybe we should at least add a valid test case for them. I added few of them as I was implementing the formatting.

Test Plan

CI should pass

@github-actions github-actions bot added A-Project Area: project A-Parser Area: parser A-Formatter Area: formatter L-Grit Language: GritQL labels Nov 5, 2024
Copy link

codspeed-hq bot commented Nov 5, 2024

CodSpeed Performance Report

Merging #4472 will improve performances by 6.23%

Comparing feat/grit-formatter-parser (8c533a8) with main (50093fb)

Summary

⚡ 1 improvements
✅ 98 untouched benchmarks

Benchmarks breakdown

Benchmark main feat/grit-formatter-parser Change
pure_9395922602181450299.css[uncached] 4.2 ms 4 ms +6.23%

@ematipico ematipico merged commit 043cd03 into main Nov 5, 2024
12 checks passed
@ematipico ematipico deleted the feat/grit-formatter-parser branch November 5, 2024 16:13
@arendjr
Copy link
Contributor

arendjr commented Nov 6, 2024

Nice! I think I added a bit more compiler tests than parser-only tests, since it would cover both. But yeah, we can certainly have more 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Formatter Area: formatter A-Parser Area: parser A-Project Area: project L-Grit Language: GritQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants