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

Extract setting for preference of newline placement for computation expressions #2746

Merged
merged 19 commits into from
Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion src/Fantomas.Core.Tests/ComputationExpressionSameLineTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,7 @@ match x with
| _ -> task {
// some computation here
()
}
}
"""

[<Test>]
Expand Down Expand Up @@ -677,3 +677,28 @@ myComp {
}
}
"""

[<Test>]
let ``prefer computation expression name on same line, with trivia`` () =
formatSourceString
false
"""
let t =
//
task {
let! thing = otherThing ()
return 5
}
"""
config
|> prepend newline
|> should
equal
"""
let t =
//
task {
let! thing = otherThing ()
return 5
}
"""
20 changes: 5 additions & 15 deletions src/Fantomas.Core/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@ let genExpr (e: Expr) =
onlyIf
(isMultiline
&& ctx.Config.MultiLineLambdaClosingNewline
&& not (ctx.Config.ExperimentalStroustrupStyle && node.Lambda.Expr.IsStroustrupStyleExpr))
&& not (isStroustrupExpr ctx.Config node.Lambda.Expr))
sepNln
ctx)
+> genSingleTextNode node.ClosingParen
Expand Down Expand Up @@ -1072,9 +1072,7 @@ let genExpr (e: Expr) =
+> onlyIfCtx
(fun ctx ->
ctx.Config.MultiLineLambdaClosingNewline
&& (not (
ctx.Config.ExperimentalStroustrupStyle && lambdaNode.Expr.IsStroustrupStyleExpr
)))
&& (not ((isStroustrupExpr ctx.Config lambdaNode.Expr))))
sepNln
+> genSingleTextNode appParen.Paren.ClosingParen
| _ ->
Expand Down Expand Up @@ -1922,7 +1920,7 @@ let genClause (isLastItem: bool) (node: MatchClauseNode) =
ctx)

let genPatAndBody ctx =
if ctx.Config.ExperimentalStroustrupStyle && node.BodyExpr.IsStroustrupStyleExpr then
if (isStroustrupExpr ctx.Config node.BodyExpr) then
josh-degraw marked this conversation as resolved.
Show resolved Hide resolved
let startColumn = ctx.Column
(genPatInClause node.Pattern +> atIndentLevel false startColumn genWhenAndBody) ctx
else
Expand Down Expand Up @@ -2181,9 +2179,7 @@ let genAppWithLambda sep (node: ExprAppWithLambdaNode) =
| Choice1Of2 lambdaNode ->
genSingleTextNode node.OpeningParen
+> (genLambdaWithParen lambdaNode |> genNode lambdaNode)
+> onlyIf
(not (ctx.Config.ExperimentalStroustrupStyle && lambdaNode.Expr.IsStroustrupStyleExpr))
sepNln
+> onlyIf (not (isStroustrupExpr ctx.Config lambdaNode.Expr)) sepNln
+> genSingleTextNode node.ClosingParen
| Choice2Of2 matchLambdaNode ->
genSingleTextNode node.OpeningParen
Expand All @@ -2204,13 +2200,7 @@ let genAppWithLambda sep (node: ExprAppWithLambdaNode) =
(genSingleTextNode node.OpeningParen
+> (genLambdaWithParen lambdaNode |> genNode lambdaNode))
(fun isMultiline ->
onlyIf
(isMultiline
&& not (
ctx.Config.ExperimentalStroustrupStyle
&& lambdaNode.Expr.IsStroustrupStyleExpr
))
sepNln
onlyIf (isMultiline && not (isStroustrupExpr ctx.Config lambdaNode.Expr)) sepNln
+> genSingleTextNode node.ClosingParen)
| Choice2Of2 matchLambdaNode ->
(genSingleTextNode node.OpeningParen
Expand Down
65 changes: 42 additions & 23 deletions src/Fantomas.Core/Context.fs
Original file line number Diff line number Diff line change
Expand Up @@ -752,14 +752,31 @@ let sepSpaceOrDoubleIndentAndNlnIfExpressionExceedsPageWidth expr (ctx: Context)
expr
ctx

let isStroustrupApplicable isStroustrupContext (node: Node) (ctx: Context) =
ctx.Config.ExperimentalStroustrupStyle
&& isStroustrupContext
&& Seq.isEmpty node.ContentBefore

let isNamedComputationAndPreferSameLine expr (ctx: Context) =
match expr with
| Expr.NamedComputation _ when ctx.Config.PreferComputationExpressionNameOnSameLine -> true
let isStroustrupExpr (config: FormatConfig) (e: Expr) =
let isStroustrupEnabled = config.MultilineBracketStyle = ExperimentalStroustrup

match e with
| Expr.Record node when isStroustrupEnabled ->
match node.Extra with
| RecordNodeExtra.Inherit _
| RecordNodeExtra.With _ -> false
| RecordNodeExtra.None -> true
| Expr.AnonRecord node when isStroustrupEnabled ->
match node.CopyInfo with
| Some _ -> false
| None -> true
| Expr.ArrayOrList _ when isStroustrupEnabled -> true
| Expr.NamedComputation _ when config.PreferComputationExpressionNameOnSameLine -> true
josh-degraw marked this conversation as resolved.
Show resolved Hide resolved
| _ -> false

let isStroustrupApplicable isStroustrupContext (node: Node) =
isStroustrupContext && not node.HasContentBefore

let isStroustrupType (config: FormatConfig) (t: Type) =
let isStroustrupEnabled = config.MultilineBracketStyle = ExperimentalStroustrup

match t with
| Type.AnonRecord _ when isStroustrupEnabled -> true
| _ -> false

let sepSpaceOrIndentAndNlnIfExceedsPageWidthUnless condition f (ctx: Context) =
Expand All @@ -769,22 +786,23 @@ let sepSpaceOrIndentAndNlnIfExceedsPageWidthUnless condition f (ctx: Context) =
sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth f ctx

let sepSpaceOrIndentAndNlnIfExceedsPageWidthUnlessStroustrup isStroustrup f (node: Node) (ctx: Context) =
sepSpaceOrIndentAndNlnIfExceedsPageWidthUnless (isStroustrupApplicable isStroustrup node ctx) f ctx
sepSpaceOrIndentAndNlnIfExceedsPageWidthUnless (isStroustrupApplicable isStroustrup node) f ctx

let sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidthUnlessStroustrup f (expr: Expr) =
sepSpaceOrIndentAndNlnIfExceedsPageWidthUnlessStroustrup expr.IsStroustrupStyleExpr (f expr) (Expr.Node expr)
let sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidthUnlessStroustrup f (expr: Expr) (ctx: Context) =
sepSpaceOrIndentAndNlnIfExceedsPageWidthUnlessStroustrup
(isStroustrupExpr ctx.Config expr)
(f expr)
(Expr.Node expr)
ctx

let sepSpaceOrIndentAndNlnIfTypeExceedsPageWidthUnlessStroustrup f (t: Type) =
sepSpaceOrIndentAndNlnIfExceedsPageWidthUnlessStroustrup t.IsStroustrupStyleType (f t) (Type.Node t)
let sepSpaceOrIndentAndNlnIfTypeExceedsPageWidthUnlessStroustrup f (t: Type) (ctx: Context) =
sepSpaceOrIndentAndNlnIfExceedsPageWidthUnlessStroustrup (isStroustrupType ctx.Config t) (f t) (Type.Node t) ctx

let sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidthUnlessSameLinePreferred f (expr: Expr) (ctx: Context) =
let isStroustrup =
isStroustrupApplicable expr.IsStroustrupStyleExpr (Expr.Node expr) ctx
isStroustrupApplicable (isStroustrupExpr ctx.Config expr) (Expr.Node expr)

sepSpaceOrIndentAndNlnIfExceedsPageWidthUnless
(isNamedComputationAndPreferSameLine expr ctx || isStroustrup)
(f expr)
ctx
sepSpaceOrIndentAndNlnIfExceedsPageWidthUnless isStroustrup (f expr) ctx

let autoNlnIfExpressionExceedsPageWidth expr (ctx: Context) =
expressionExceedsPageWidth
Expand Down Expand Up @@ -925,26 +943,27 @@ let addParenIfAutoNln expr f =

let autoIndentAndNlnExpressUnlessSameLinePreferred f (e: Expr) (ctx: Context) =
let shouldUseStroustrup =
isStroustrupApplicable e.IsStroustrupStyleExpr (Expr.Node e) ctx
isStroustrupApplicable (isStroustrupExpr ctx.Config e) (Expr.Node e)

if shouldUseStroustrup || isNamedComputationAndPreferSameLine e ctx then
if shouldUseStroustrup then
f e ctx
else
indentSepNlnUnindent (f e) ctx

let autoIndentAndNlnTypeUnlessStroustrup f (t: Type) (ctx: Context) =
let shouldUseStroustrup =
isStroustrupApplicable t.IsStroustrupStyleType (Type.Node t) ctx
isStroustrupApplicable (isStroustrupType ctx.Config t) (Type.Node t)

if shouldUseStroustrup then
f t ctx
else
autoIndentAndNlnIfExpressionExceedsPageWidth (f t) ctx

let autoIndentAndNlnIfExpressionExceedsPageWidthUnlessSameLinePreferred f (e: Expr) (ctx: Context) =
let isStroustrup = isStroustrupApplicable e.IsStroustrupStyleExpr (Expr.Node e) ctx
let isStroustrup =
isStroustrupApplicable (isStroustrupExpr ctx.Config e) (Expr.Node e)

if isNamedComputationAndPreferSameLine e ctx || isStroustrup then
if isStroustrup then
f e ctx
else
autoIndentAndNlnIfExpressionExceedsPageWidth (f e) ctx
Expand Down
3 changes: 2 additions & 1 deletion src/Fantomas.Core/Context.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,8 @@ val leadingExpressionResult:
val leadingExpressionIsMultiline:
leadingExpression: (Context -> Context) -> continuationExpression: (bool -> Context -> 'a) -> ctx: Context -> 'a

val isStroustrupExpr: config: FormatConfig -> e: Expr -> bool

/// try and write the expression on the remainder of the current line
/// add an indent and newline if the expression is longer
val autoIndentAndNlnIfExpressionExceedsPageWidth: expr: (Context -> Context) -> ctx: Context -> Context
Expand Down Expand Up @@ -257,7 +259,6 @@ val sepNlnWhenWriteBeforeNewlineNotEmpty: (Context -> Context)
val sepSpaceUnlessWriteBeforeNewlineNotEmpty: ctx: Context -> Context
val autoIndentAndNlnWhenWriteBeforeNewlineNotEmpty: f: (Context -> Context) -> ctx: Context -> Context
val addParenIfAutoNln: expr: Expr -> f: (Expr -> Context -> Context) -> (Context -> Context)
val isStroustrupApplicable: isStroustrupContext: bool -> node: Node -> ctx: Context -> bool

val autoIndentAndNlnExpressUnlessSameLinePreferred:
f: (Expr -> Context -> Context) -> e: Expr -> ctx: Context -> Context
Expand Down
19 changes: 0 additions & 19 deletions src/Fantomas.Core/SyntaxOak.fs
Original file line number Diff line number Diff line change
Expand Up @@ -382,11 +382,6 @@ type Type =
| Or n -> n
| LongIdentApp n -> n

member e.IsStroustrupStyleType: bool =
match e with
| AnonRecord _ -> true
| _ -> false

/// A pattern composed from a left hand-side pattern, a single text token/operator and a right hand-side pattern.
type PatLeftMiddleRight(lhs: Pattern, middle: Choice<SingleTextNode, string>, rhs: Pattern, range) =
inherit NodeBase(range)
Expand Down Expand Up @@ -1727,20 +1722,6 @@ type Expr =
| Typar n -> n
| Chain n -> n

member e.IsStroustrupStyleExpr: bool =
match e with
| Expr.Record node ->
match node.Extra with
| RecordNodeExtra.Inherit _
| RecordNodeExtra.With _ -> false
| RecordNodeExtra.None -> true
| Expr.AnonRecord node ->
match node.CopyInfo with
| Some _ -> false
| None -> true
| Expr.ArrayOrList _ -> true
| _ -> false

member e.HasParentheses: bool =
match e with
| Expr.Paren _ -> true
Expand Down