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.

Sign up for GitHub

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

Jump to bottom

feat(grainfmt)!: Implement new formatter #1976

Merged
merged 48 commits into from
Feb 19, 2024
Merged

feat(grainfmt)!: Implement new formatter #1976

merged 48 commits into from
Feb 19, 2024

Conversation

ospencer
Copy link
Member

@ospencer ospencer commented Jan 23, 2024 •
edited by phated
Loading

The goal of this new formatter is to be more maintainable and make it easier to implement new features.

Closes #1610
Closes #1627
Closes #1634
Closes #1635
Closes #1644
Closes #1654
Closes #1828
Closes #1848
Closes #1516

@ospencer ospencer self-assigned this Jan 23, 2024
@ospencer
Copy link
Member Author

ospencer commented Jan 23, 2024 •
edited
Loading

I didn't try to land the Add list AST node commit as its own PR because, well, that would require getting the tests to pass in the old formatter.

If you want to see the changes to how the stdlib is now formatted, you can look at this commit.

alex-snezhko
alex-snezhko reviewed Jan 23, 2024
View reviewed changes
Copy link
Member

@alex-snezhko alex-snezhko left a comment

Choose a reason for hiding this comment

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

Incredible work! Left a few first-pass comments but looks great overall

compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/src/formatting/fmt.re Show resolved Hide resolved
spotandjake
spotandjake reviewed Jan 23, 2024
View reviewed changes
Copy link
Member

@spotandjake spotandjake left a comment

Choose a reason for hiding this comment

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

I really like some of the formatting changes here. Just a few nits about the output in a few places but otherwise looks great.

compiler/src/formatting/fmt.re Outdated Show resolved Hide resolved
compiler/test/grainfmt/data_docs.expected.gr Outdated Show resolved Hide resolved
compiler/test/grainfmt/enums.expected.gr Show resolved Hide resolved
compiler/test/grainfmt/function_params.expected.gr Show resolved Hide resolved
stdlib/marshal.gr Show resolved Hide resolved
@ospencer ospencer force-pushed the oscar/formatter branch 2 times, most recently from 3f3fcfd to cf38c4b Compare January 25, 2024 16:20
@spotandjake spotandjake added the grainfmt Issues related to the Grain formatter label Jan 26, 2024
@alex-snezhko alex-snezhko mentioned this pull request Jan 27, 2024
Closed
phated
phated reviewed Feb 10, 2024
View reviewed changes
Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

I reviewed the formatting in files changed since my last review and saw a couple more things.

compiler/test/grainfmt/patterns.expected.gr Outdated Show resolved Hide resolved
compiler/test/grainfmt/aliases.expected.gr Outdated Show resolved Hide resolved
compiler/test/grainfmt/records.expected.gr Outdated Show resolved Hide resolved
compiler/test/grainfmt/records.expected.gr Outdated Show resolved Hide resolved
This was referenced Feb 19, 2024
Open
Open
Open
Open
phated
phated approved these changes Feb 19, 2024
View reviewed changes
compiler/test/grainfmt/comments.expected.gr Show resolved Hide resolved
compiler/test/grainfmt/comments.expected.gr Show resolved Hide resolved
compiler/test/grainfmt/comments.expected.gr Show resolved Hide resolved
compiler/test/grainfmt/comments.expected.gr Show resolved Hide resolved
compiler/test/grainfmt/comments.expected.gr Show resolved Hide resolved
compiler/test/grainfmt/comments.expected.gr Show resolved Hide resolved
stdlib/marshal.gr Show resolved Hide resolved
compiler/src/formatting/fmt.re Show resolved Hide resolved
compiler/src/formatting/fmt.re Show resolved Hide resolved
compiler/src/formatting/fmt.re Show resolved Hide resolved
@phated phated mentioned this pull request Feb 19, 2024
Open
@ospencer ospencer added this pull request to the merge queue Feb 19, 2024
Merged via the queue into main with commit 1568aa0 Feb 19, 2024
12 checks passed
@ospencer ospencer deleted the oscar/formatter branch February 19, 2024 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alex-snezhko alex-snezhko alex-snezhko left review comments

@spotandjake spotandjake spotandjake left review comments

@marcusroberts marcusroberts marcusroberts left review comments

@phated phated phated approved these changes

@peblair peblair Awaiting requested review from peblair peblair is a code owner

@jozanza jozanza Awaiting requested review from jozanza jozanza is a code owner

Assignees

@ospencer ospencer

Labels
grainfmt Issues related to the Grain formatter
Projects
No open projects
Grain v0.6.0
Status: Done
Milestone
No milestone
5 participants
@ospencer @phated @marcusroberts @spotandjake @alex-snezhko