-
Notifications
You must be signed in to change notification settings - Fork 484
Fix functor module type result-signature brace formatting #8519
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,8 +52,7 @@ module M3: { | |
| include M' | ||
| } | ||
|
|
||
| module G0: (X: {}) => | ||
| { | ||
| module G0: (X: {}) => { | ||
| module N': { | ||
| let x: int | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| module Make: Pattern => { | ||
| let fmt: event => string | ||
| } | ||
|
|
||
| module Curried: (A, B) => { | ||
| let x: int | ||
| } | ||
|
|
||
| module Nested: (A, B) => { | ||
| let y: int | ||
| } | ||
|
|
||
| module Empty: () => {} | ||
|
|
||
| module ResultIdent: A => B | ||
|
|
||
| module ResultWith: A => (B with type t = int) | ||
|
|
||
| module ThisIsAVeryLongFunctorModuleTypeNameThatExceedsTheEightyCharacterLimit: Pattern => { | ||
| let fmt: event => string | ||
| } | ||
|
|
||
| module SecondVeryLongFunctorModuleTypeNameWithMultipleShortParameters: (A, B) => { | ||
| let x: int | ||
| } | ||
|
|
||
| module YetAnotherVeryLongFunctorModuleTypeNameHere: SomeVeryLongParameterModuleTypeName => | ||
| SomeVeryLongResultingModuleTypeName |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| module Make: Pattern => { | ||
| let fmt: event => string | ||
| } | ||
|
|
||
| module Curried: (A, B) => { | ||
| let x: int | ||
| } | ||
|
|
||
| module Nested: A => B => { | ||
| let y: int | ||
| } | ||
|
|
||
| module Empty: () => {} | ||
|
|
||
| module ResultIdent: A => B | ||
|
|
||
| module ResultWith: A => (B with type t = int) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MavenRain can you add module name with a longer name as well (more than 80 characters in one line) to test how the formatter handles that case?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added three long-name cases (each over 80 columns) to module ThisIsAVeryLongFunctorModuleTypeNameThatExceedsTheEightyCharacterLimit: Pattern => {
let fmt: event => string
}
module SecondVeryLongFunctorModuleTypeNameWithMultipleShortParameters: (A, B) => {
let x: int
}
module YetAnotherVeryLongFunctorModuleTypeNameHere: SomeVeryLongParameterModuleTypeName =>
SomeVeryLongResultingModuleTypeNameHow the formatter handles them:
The full corpus regeneration shows no other golden moved, and the printer + idempotency roundtrip suite still passes; the new cases are idempotent. Reverting the fix breaks the new golden on exactly the two long-name signature cases, so they do exercise the changed code path. |
||
|
|
||
| module ThisIsAVeryLongFunctorModuleTypeNameThatExceedsTheEightyCharacterLimit: Pattern => { | ||
| let fmt: event => string | ||
| } | ||
|
|
||
| module SecondVeryLongFunctorModuleTypeNameWithMultipleShortParameters: (A, B) => { | ||
| let x: int | ||
| } | ||
|
|
||
| module YetAnotherVeryLongFunctorModuleTypeNameHere: SomeVeryLongParameterModuleTypeName => SomeVeryLongResultingModuleTypeName | ||
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.
sorry I should be more explicit @MavenRain, can you also add a similar test here? Just need to be sure that both
.resand.resiare handled correctly.