fsharp / fslang-design

RFCs and docs related to the F# language design process, see https://github.com/fsharp/fslang-suggestions to submit ideas
518 stars 144 forks source link

[style-guide] Units of measure should format like expressions #660

Closed sonicbhoc closed 2 years ago

sonicbhoc commented 2 years ago

There are no guidelines for units of measure. Due to this, the Fantomas team does not have guidance for formatting them, which leads to oddities such as those seen in fsprojects/fantomas#2207. Please add guidelines for formatting units of measure.

nojaf commented 2 years ago

Hello @dsyme, the problem space here is that Fantomas currently produces this:

type Test =
    { WorkHoursPerWeek: uint<hr / (staff weeks)> }
    static member create = { WorkHoursPerWeek = 40u<hr/(staff weeks)> }

Where there are spaces around the / in the type definition but not when used in the measure instance. Any thoughts?

smoothdeveloper commented 2 years ago

🎉

type Test =
    { WorkHoursPerWeek: uint< hr / (staff weeks) > }
    static member create = { WorkHoursPerWeek = 40u< hr / (staff weeks) > }

🚀

type Test =
    { WorkHoursPerWeek: uint<hr / (staff weeks)> }
    static member create = { WorkHoursPerWeek = 40u<hr / (staff weeks)> }

👀

type Test =
    { WorkHoursPerWeek: uint<hr/(staff weeks)> }
    static member create = { WorkHoursPerWeek = 40u<hr/(staff weeks)> }
nojaf commented 2 years ago

Hi @smoothdeveloper, I appreciate your idea of creating a poll, but I think we should try to keep this a constructive debate with arguments about why a certain style fits better in the overall story of the style guide rather than just voting away.

auduchinok commented 2 years ago

The third variant may seem a bit "prettier" with shorter measure types, but the second one (i.e. with spaces around /) is more inline with the rest of the guide for various language parts so I'd vote for it.

dsyme commented 2 years ago

The third variant may seem a bit "prettier" with shorter measure types, but the second one (i.e. with spaces around /) is more inline with the rest of the guide for various language parts so I'd vote for it.

I agree, this looks right to me

dsyme commented 2 years ago

I agree with the 🚀 suggestion above as the clarification

@nojaf this seems to come sufficiently under "Always use white space around binary arithmetic expressions" in the style guide. I suppose a specific clarification that this also applies to units of measures in types and constant annotations would be reasonable.

sonicbhoc commented 2 years ago

Looks good to me.