-
Notifications
You must be signed in to change notification settings - Fork 430
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
WIP Prettier Printing of function calls DO NOT MERGE #2679
base: master
Are you sure you want to change the base?
Conversation
type variant = | ||
| X(int, int) /* End of line on X */ | ||
| Y(int, int); /* End of line on Y */ /* Comment on entire type def for variant */ | ||
type variant = | X(int, int) /* End of line on X */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a regression obviously.
n + tmp; | ||
| None => 20 | ||
}; | ||
let result = switch (None) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the right idea.
| Y(_) => "result of Y" /* End of arrow and Y line */ | ||
}; | ||
|
||
type optionalTuple = | OptTup( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regression
), | ||
), | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right idea below
1, /*eol1*/ | ||
2 /* eol2 */ | ||
); | ||
let y = TwoArgsConstructor( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right idea below
This demonstrates how to achieve prettier printing of function calls. Sending a PR just for discussion. It is based on a really old commit of Reason, but shouldn't be too hard to rebase and fix the regressions.
Instead of:
It will print:
In its current state it also applies the same convention to other constructs which should not be wrapped in that manner - it's just a matter of going through and telling the printer which style to use in each case.
I included some test output changes which demonstrate both some improvements and some regressions in printing.