[style-guide] Stroustrup bracket style #706
Comments
josh-degraw
changed the title
|
I'm a big fan of style. Particularly Elmish benefits greatly but other places do as well, such as Expecto. The shuffling of horizontal as @josh-degraw becomes irritating when needing to do things like:
Currently the Formatting Guidelines seems inconsistent when it prefers StroustrupAllman, when it prefers something like Pico/Lisp. While some of these are more "Allman" there's a lot of ergonomic similarities between to two so I'm going to include them. Places it seems to prefer StroustrupFormatting application expressionsExample: // ✔️ OK
someTupledFunction (
478815516,
"A very long string making all of this multi-line",
1515,
false
)
// OK, but formatting tools will reformat to the above
someTupledFunction
(478815516,
"A very long string making all of this multi-line",
1515,
false)Formatting union case expressions// ✔️ OK
let tree1 =
BinaryNode(
BinaryNode (BinaryValue 1, BinaryValue 2),
BinaryNode (BinaryValue 3, BinaryValue 4)
)Formatting quoted expressions// ✔️ OK
<@
let f x = x + 10
f 20
@>
// ❌ Not OK
<@ let f x = x + 10
f 20
@>Formatting computation expression operationslet myNumber =
math {
addOne
addOne
addOne
subtractOne
divideBy 2
multiplyBy 10
}Places it seems to not prefer themFormatting lambda expressionsExample: // ✔️ OK
Target.create "Build" (fun ctx ->
// code
// here
())Formatting record expressions// ✔️ OK
let newState =
{ state with
Foo = Some { F1 = 0; F2 = "" } }
// ❌ Not OK, code formatters will reformat to the above by default
let newState =
{
state with
Foo =
Some {
F1 = 0
F2 = ""
}
}Formatting object expressions// ✔️ OK
let comparer =
{ new IComparer<string> with
member x.Compare(s1, s2) =
let rev (s: String) = new String (Array.rev (s.ToCharArray()))
let reversed = rev s1
reversed.CompareTo (rev s2) }Places it seems conflictedFormatting list and array expressions// ✔️ OK
let pascalsTriangle =
[| [| 1 |]
[| 1; 1 |]
[| 1; 2; 1 |]
[| 1; 3; 3; 1 |]
[| 1; 4; 6; 4; 1 |]
[| 1; 5; 10; 10; 5; 1 |]
[| 1; 6; 15; 20; 15; 6; 1 |]
[| 1; 7; 21; 35; 35; 21; 7; 1 |]
[| 1; 8; 28; 56; 70; 56; 28; 8; 1 |] |]
//vs
let daysOfWeek includeWeekend =
[
"Monday"
"Tuesday"
"Wednesday"
"Thursday"
"Friday"
if includeWeekend then
"Saturday"
"Sunday"
]Formatting record declarations // ✔️ OK
type PostalAddress =
{ Address: string
City: string
Zip: string }
member x.ZipAndCity = $"{x.Zip} {x.City}"
// ❌ Not OK, code formatters will reformat to the above by default
type PostalAddress = {
Address: string
City: string
Zip: string
}
with
member x.ZipAndCity = $"{x.Zip} {x.City}"
endhowever when there are doc-comments, Allman style is preferred // ✔️ OK
type PostalAddress =
{
/// The address
Address: string
/// The city
City: string
/// The zip code
Zip: string
}
/// Format the zip code and the city
member x.ZipAndCity = $"{x.Zip} {x.City}" |
|
My personal preferred syntax for records is: type PostalAddress = {
Address: string
City: string
Zip: string
} with
member x.ZipAndCity = $"{x.Zip} {x.City}"( Anyone else? |
|
Looking at a personal project I've been working on recently, it appears that I prefer to have type Config = {
Pin: string
MpvPath: string
NpvrBaseAddress: Uri
TempDirPath: string
PlaybackSpeed: float
}
with
static member Create(config: IConfiguration) = {
Pin = config.Item("Pin")
MpvPath = config.Item("MpvPath")
NpvrBaseAddress =
config.Item("NpvrBaseAddress")
|> Option.tryParseUri UriKind.Absolute
|> Option.defaultValue (Uri("http://localhost:8866"))
TempDirPath = config.Item("TempDirPath")
PlaybackSpeed =
config.Item("PlaybackSpeed")
|> Option.tryParse
|> Option.defaultValue 1.0
} |
|
Hello @josh-degraw! Many thanks for bringing this up in I'd strongly prefer to have a more high-level conversation about this first. type PostalAddress =
{ Address: string
City: string
Zip: string }
type PostalAddress =
{
Address: string
City: string
Zip: string
}
type PostalAddress = {
Address: string
City: string
Zip: string
}That just seems like a step back in getting the F# community to format code all in the same way. As @TheAngryByrd rightfully pointed out, there are places in the style guide where An example of function invocation: someTupledFunction (
478815516,
"A very long string making all of this multi-line",
1515,
false
)This will result in a single Whereas an example of a record instance can be perceived as "non-self-contained". let x = {
X = 1
Y = 2
}The record instance ( Another example let x =
let y = "some string"
{ X = 1
Y = 2 }
|> printfn "%A"In this case, the same That is one of my major concerns with this. When should it work? What is the heuristic? I really would like to address these higher-level questions first instead of this becoming a thread of "I like detail X" or "I like detail Y". Liking anything doesn't really add anything to this discussion to me. |
|
Not sure how you could not pull it off in that example. I reworked it and tested it in the AST on Fantomas website, and, with exception of range numbers, ended up with the same exact AST. let x =
let y = "some string"
{
X = 1
Y = 2
}
|> printfn "%A"I find myself using that style often when I'm needing to pipe the result of a Computation Expression into a function pipeline. let x =
let y = "some string"
printfn "%A" {
X = 1
Y = 2
} |
Yes, are dancing around the problem there. My point indeed was that there are scenario's in general where you cannot apply Stroustrup. printfn "%A" {
X = 1
Y = 2
}This is almost indistinguishable from a computation expression that takes a parameter. |
|
Well, if the issue is the visual ambiguity between function application of a record constructor and a computation expression, you could make use of (and possibly enforce, though I don't know what issues that may cause) the backwards pipe operator. printfn "%A" <| {
X = 1
Y = 2
} |
Are you suggesting that we shouldn't be able to use Stroustrup in that example? That's exactly how I would suggest to format that example. I'm not sure I see the major problem with this; they're all functions in the end right? As in, especially in the case of a parameterized computation expression, that's essentially just a function that happens to accept the computation expression as the second parameter. |
Exactly, because it is visually too close to computation expression. My original point I tried to make with: { X = 1
Y = 2 }
>=> printfn "%A"(Note that I'm using a different operator I think it would be best to limit it to what covers 80% of it and has absolutely no ambiguity. let items = [ {
X = 1
Y = 2
} {
X = 3
Y = 4
} {
X = 5
Y = 6
} ]As some middle ground I would format it like: let items = [
{
X = 1
Y = 2
}
{
X = 3
Y = 4
}
{
X = 5
Y = 6
}
]Falling back Stroupstrup to Allman when edge cases arise. |
|
So, am I understanding correctly that when you say it is not possible to do Stroupstrup because there is no "parent line" where the opening brace can be added to the end, and any attempt would end up with Allman style? In my opinion (which may be wrong), in the even there is no "parent line," Allman and Stroupstrup are identical. Thus, this idea of falling back to Allman style in certain situations where strict Stroupstrup is impossible or makes the reading and editing more difficult makes sense to me. Also, in these given examples, where the record names are short and there are few of them, I'd tend to single line the constructors, reserving the block approach for long names and/or many records, as the advantage for using block styles are when the names are long, in order to prevent long lines, and when there are more than a couple properties, so that deleting, adding, and reordering is simpler. |
Yikes, yeah that first example physically hurts 🤣 For what it's worth, when I originally opened the fantomas issue, the main use case I had in mind was that I disliked the "classic" style, and I wanted to use Stroustrup for type declarations, I hadn't really thought about a heuristic on when else to use it so I didn't really mention anything else. Most other times, Allman style works perfectly fine, and I think it's a very fair compromise to use in the cases like this where Stroustrup doesn't work as well. |
Yes, and I totally understand you here. In the past, a principle got approved where the guide vaguely said: "avoid vanity alignment". This principle stands but it was quickly used as a stick to club the Fantomas maintainers to death. People opened issues where the guide really didn't go into detail, all under the pretence of a vague rule. That is why I really would like to have a clear scope of where the guide recommends you can use Stroustrup. Everything not mentioned should (for safety reasons) be considered to fall back to Allman. |
|
I ran into a situation for Stroustrup related to fsprojects/fantomas#2513 and fsprojects/fantomas#2515 that I think is worth discussing here: Given this input in the 'classic' style: let compareThings (first: Thing) (second: Thing) =
first = { second with
Foo = first.Foo
Bar = first.Bar }Currently, we seem to format Stroustrup let compareThings (first: Thing) (second: Thing) =
first = { second with
Foo = first.Foo
Bar = first.Bar
}To me, this feels like a more "correct" Stroustrup style for the expression: let compareThings (first: Thing) (second: Thing) =
first = {
second with
Foo = first.Foo
Bar = first.Bar
}This would also be the only format of the 3 (as far as I can tell) that could fully and directly utilize a single additional tab stop for each level. |
|
Another situation worth discussing is types with accessibility modifiers and additional members, e.g. fsprojects/fantomas#2511. Given the following input type NonEmptyList<'T> =
private
{ List: 'T list }
member this.Head = this.List.Head
member this.Tail = this.List.Tail
member this.Length = this.List.LengthCurrently, fantomas appears to want to fall back to Allman style: type NonEmptyList<'T> =
private
{
List: 'T list
}
member this.Head = this.List.Head
member this.Tail = this.List.Tail
member this.Length = this.List.LengthMy personal preference would be something like this: type NonEmptyList<'T> = private {
List: 'T list
}
with
member this.Head = this.List.Head
member this.Tail = this.List.Tail
member this.Length = this.List.Lengthbut I know you hate having to pull out the type NonEmptyList<'T> =
private {
List: 'T list
}
member this.Head = this.List.Head
member this.Tail = this.List.Tail
member this.Length = this.List.LengthIMHO any option is probably fine as long as it's valid code, but it's worth discussing here for the sake of having a formal specification. |
|
Apropo of my last comment, I think a discussion about pros and cons of allowing the If we have a type formatted via Stroustrup: type Person = {
FirstName: string
LastName: string
}And later we want to add an additional member e.g. a type Person = {
FirstName: string
LastName: string
}
+with
+ member this.FullName =
+ this.FirstName + " " + this.LastNameBut if we enforce falling back to Allman when additional members are present, we end up with a much larger diff because every line of the type needed to be adjusted: -type Person = {
- FirstName: string
- LastName: string
-}
+type Person =
+ {
+ FirstName: string
+ LastName: string
+ }
+ member this.FullName =
+ this.FirstName + " " + this.LastNameSo in short, allowing the |
|
Not exactly style but it might be worth figuring out if the |
I agree, it would be pretty nice if that was possible, if this was acceptable by the parser: type Person = {
FirstName: string
LastName: string
}
member this.FullName =
this.FirstName + " " + this.LastNameAlthough I don't know nearly enough about the compiler to know how big of a change that would be. |
first = {
second with
Foo = first.Foo
Bar = first.Bar
}This just really gives me computation expression vibes. fn arg1 {
X = x
Y = y
}This is visually just too close to me for seq {
...
...
}Regarding the type Person = {
FirstName: string
LastName: string
} with
member this.FullName =
this.FirstName + " " + this.LastNameThat wouldn't be overly hard to implement and might be a good trade-off. |
Personally I would have no issues with that. The keys for me are just not needing to completely reformat the type in order to extend it and keeping the diff small, and this checks both boxes for me, so especially if it's easier to implement I'm all for it👍🏻
You've mentioned this concern a few times, and I'd be interested to see if the community has strong opinions about it one way or another. On one hand I totally understand, but on the other hand my gut still likes it for the visual consistency of the brackets haha. I suspect that might come down to a personal preference thing tbh, because the general case of function parameter application I'm inclined to prefer this way, but I do understand the desire to keep computation expressions a "special circumstance." I will say though that I do think that in this specific situation it should be clear enough that it's not a computation expression since there's no actual "expression" name.
That's about what I expected haha. Out of sheer curiosity, do you have a ballpark estimate of how difficult of a task you think that would be? |
Yes, please let us know.
Really hard to say, somewhere between two hours and two weeks I guess. The lexer can be quite challenging. A small tweak there could have quite the butterfly effect. |
|
Just to say that, from a style-guide perspective, I'm ok with there being an end-to-end Stroustrup option, though I realise it's a big job and hits some technical limitations. @nojaf I can't just "accept" because this is such a big thing? What process would you like to follow? I'm thinking some criteria are
|
|
Hello @dsyme, I think this is a sensible approach.
Everything not specifically covered in the guide would not be considered a gap in Fantomas. |
|
I haven't followed the Stroustrup discussion closely, so I'm not sure if this is timely, but I just want to point out that Stroustrup style compiles for quotations. For example, here is an Unquote assertion: test <@
output
|> Array.pairwise
|> Array.forall (fun (grp1, grp2) -> predicate (Array.last grp1) (Array.head grp2) = false)
@>Whether this style is desirable is another matter. Allman also works, but of course adds a level of indentation: test
<@
output
|> Array.pairwise
|> Array.forall (fun (grp1, grp2) -> predicate (Array.last grp1) (Array.head grp2) = false)
@> |
|
Ahem, chiming in with my controversial set of opinions... but I've found myself using a pattern in particular in places like function definitions whenever the content is wrapped in a CE, be it let doStuffAsync myParameter = async {
let! someOtherThing = giveMeSomeOtherThingAsync()
return myParameter + someOtherThing
}What I'd like to know is whether this thing above 👆 would be regarded as "compliant" with the Stroustrup bracket style, or is it something different? Cause you see, I don't really mind stuff like in the example given by @TheAngryByrd: let myNumber =
math {
addOne
addOne
addOne
subtractOne
divideBy 2
multiplyBy 10
}but it feels "weird" in many instances. One might expect to not have to tab the whole content just because the function body is wrapped in a CE. Thing like below 👇 keeps things terse and neat. Imho, this improves readability without it being at the expense of adding a new line and tabbing the whole body: let myNumber = math {
addOne
addOne
addOne
subtractOne
divideBy 2
multiplyBy 10
}I guess this is probably a very unpopular opinion, but thought this might be a wee related. |
|
To me, I get the same benefits in either style that @natalie-o-perret describes so I don't particularly have a preference. That being said , the more inline version does not allow piping within the same expression : let foo () = async {
return 42
}
|> Async.RunSynchronouslyWould throw an error while let foo () =
async {
return 42
}
|> Async.RunSynchronouslyIs allowed. |
|
Yeah, I've kind of gone back and forth on this, initially I thought I preferred the CE name on the same line as the |
|
Edit: After some other discussions, consensus is to just keep using |
|
With the merge of dotnet/docs#33171, I feel like this issue can be considered resolved. If additional Stroustrup specific style-guide changes need to be proposed (e.g. for Elmish styles), I think we're at a point where they should be their own proposals. If there are no objections, I'll go ahead and close this issue. |
In the Fantomas repo, I opened an issue a while ago proposing to allow Fantomas to format F# code using Stroustrup style, which essentially just places the open brace on the same line as the binding, and the closing brace on its own line.
Although originally decided against, the issue was reopened and some discussion and development has happened there and in the Fantomas discord. Along with the technical and ergonomic arguments in its favor, because this style is much more common in other programming languages than "classic" F# code style, part of the reason this was reopened in Fantomas was because there has been some agreement that it could help make F# code feel more natural to newcomers, and that impact would have more effect if it was represented in the F# official style guide.
Since it looks like there is the possibility that this could potentially be considered an officially "blessed" F# coding style, @nojaf has suggested that it would be better to discuss specifics and have a discussion here, rather than just in the Fantomas space, since Fantomas aims to format F# code according to the F# style guides rather than just arbitrary preferences.
One of the main technical arguments in favor of Stroustrup style is that it allows for easily shifting lines around in an object. For example, to rearrange any member of this type requires both vertical and horizontal context, because the brackets are on the same line as two of the members:
Whereas with Stroustrup style, if you needed to add, move, or remove a member in this type definition, you can insert a line anywhere in the object without problem. In my editor, that means I can even quickly move lines via
Alt+UporAlt+Down, which I couldn't do with the "classic" style:There are a few situations where this style is already used in F#, specifically Elmish code, but until now it hasn't been widely used in any official F# style guide.
There are pros and cons to this style, as there are with any formatting style, and I don't intend to list them all here myself right now, rather I wanted to formally open the discussion here to both get more community input and come up with an actual style guide for when and where this style would/could/should be used, so we can hopefully have this style in the official style guide in some capacity.
The text was updated successfully, but these errors were encountered: