Skip to content

Put long multiline constrained SRTP on separate lines.#2263

Merged
nojaf merged 2 commits into
fsprojects:masterfrom
nojaf:fix-2230
May 18, 2022
Merged

Put long multiline constrained SRTP on separate lines.#2263
nojaf merged 2 commits into
fsprojects:masterfrom
nojaf:fix-2230

Conversation

@nojaf

@nojaf nojaf commented May 17, 2022

Copy link
Copy Markdown
Contributor

Could you please review @dsyme if this is in line with the style guide?
I do notice that Fantomas doesn't put a space before the colon in when ^T :.
Is this by design for constraints?

Fixes #2230.

@nojaf nojaf requested a review from dsyme May 17, 2022 07:53
@nojaf nojaf linked an issue May 17, 2022 that may be closed by this pull request
3 tasks
@nojaf nojaf changed the title Put long multiline constrained SRTP on separate lines. Fixes #2230. Put long multiline constrained SRTP on separate lines. May 17, 2022
@dsyme

dsyme commented May 17, 2022

Copy link
Copy Markdown
Contributor

I do notice that Fantomas doesn't put a space before the colon in when ^T :.
Is this by design for constraints?

This is ok

when ^T: (static member (+): ^T * ^T -> ^T)
and ^T: (static member DivideByInt: ^T * int -> ^T)
and ^T: (static member Zero: ^T)
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historically I would have indented this further considering the when as part of the type , rather than part of the declaration. I'm not sure if this will cause problems. For example it could be like this:

val inline average:
    array: ^T[] -> ^T
        when ^T: (static member (+): ^T * ^T -> ^T)
        and ^T: (static member DivideByInt: ^T * int -> ^T)
        and ^T: (static member Zero: ^T)

I suppose this PR should add some examples of multi-line types (curried, tupled arguments) to check things are ok?

For example this sort of thing - https://fsprojects.github.io/fantomas-tools/#/fantomas/preview?data=N4KABGBEDGD2AmBTSAuKBtAPAYVgWwAcBLAG0XgDkBDPRACgB1IBBAN0QCcqBzRJgSgB8AXQYcGAO1ZUSYIhJLzEYKuy680VDlwCeaAHoAVdMLABaQSu1U9YIyfOWtug8dMWrLu28eebrhw9nf29Ap2tbe3dwryjfYMifDyMxSQgIAHcAC0QJbzQ6AGcAFypiomgwWjwAI04wOgBqflcwACpvXyN%2BVLz0qgl4fIaSsoqqxFr6gBEiViIkACEdAEkJYtaO%2BWKuwx7xPogBoaMC0fLK6rqOMAAtTlhXfkgAGnAoQsRi8oluQtQwOh3hBQOl0pAACTFHQEZBoSDbV7A8EQ6QkACucMByLBAAY3mCwZA1kh1gBlIgAL2QBMJEAALDiwMJkQBfWkgpmQ6GwgEI9ZIumQtGYgFAukQACMHMJkAAslQAB4AGSUyty3GKWUFEslACZcUyWWD2cjQbKoTCsZBcvAAPIAM1VEkQZOhZB1RNRMlFaHFEr1MqJAFFBo7nTSmRBICQHZAjWyZeavTzrTVYLASJ6USKsf66QBmIPgtafDjFABi8hkFEQGUULuzYOKHExCZNSa5lt58PTmabUG9GLzUbA9OL0bJBCo0EQi0QDtgHEQAAUtDQvpwBxAW226cb0qawcmUam%2BX2s8XhT6RxKwABWCdQKczucLpeIZWwDKcaBUT5rKwsB-uUsASNuYC7og7aHp2QrdmmGaXl2uZiqOABsT6QC%2Bs7zouy4AKoELCHB-gBUjAWMYEQQ6MifDBEBHukJ7Rgh55IQO17Dmhd4AOxYThb74Yg2AkP%2BhS4BIJSttAxRLjRdHQfuiZml2Z69hxV5Dr62J3gAHAJ064e%2By5ypM1wKSQ9HKR2qnwepUAXpx2m3hKACchmvnhH64CQ1FPrRVlKYSB6MXBFoOZATlaahfqjpK%2BKjthRmIMwDrFJwuB4HgVAQVBDFgExnL2Va7H9jFN48bq0pJYJ3nLmSkwVJm-mjoF1khSpx5qaVGnlShlVxXe%2BqebOaUZRwjV4M1fngU%2B%2BU2bBdkRb1jmaQN3FDbqRa1SlzAcLA6KDNMiCKNNE15a2wVgqFhXhSmq38sUzmxbpurjklCqKisDqGDkEjBkFZJZEuxQAOoLFqEH0oai1hctD09lAiIVZtb10pKj6fUqawOkQip2iRZRLsGioEMuhSFEQrV3vesOdbZ3UlUjT0vYN6OEpKmHY4qABKiBwBw8AQ-AUNPjDBVFWALGDpFKMbTp%2Bac-xPP84LlDolMHCOisGV4P8T6SpL92no9eDoiQ5QNogFZLjl3ycIYpWo4r8UGUlatLvAcoW1bSi2xw9sXVh0BZOusmcAA%2BhkkPasbCOmyz8vwa9StgpKHk8-tuh2hwqolCLYujhLcN3QnrFywKLuuXSBpYV92c2Ln%2BfFBQmvXDresG-F8dMytLPm5bpD%2B3bZQTU7vLV1Vtc1XeLARM3RAlD7Q-WwHQdbiHYdcBHHDR7H8al1LMvco9ycWqno56oGPMAGo3os8jwPI3CF9qT56fTN1dcxPVJ1XCsa6Ej1DtOeX0KxHVktTCQj9BgvzftDL%2B6RbrHz-tac%2BXpL53j1B9MBSppiwGKAAcS%2BKTcmiBKbQIQU%2BOmvdf7M0Qv1FO7M07pD1FjMBvth4ukWH5aAABrRYO9%2BFfEKHaCQZINy%2BU1nNNqik6HFX7ow5CzC0asIgHqbmc9az1iUPOYoP5cgT0QCdPGEgiCgQkMwQYZktbdzvO1a6yCf6KMRsotmair4qznswRQ3AJAQIkFAsCFJ-FlHRMuQwsASS5FKJYyyHVv6M3oUosqKiL4sKvu7HxltOASDGOwL8vxbHXFMfICx0D7ESkcQo6WaC0keNdtgzOnCh4RmVDQGo8AqCiVgFTX4OjrYJKcRAFBJsK6PWioA6ehICyJTnmQzgRBaDrBkAAaUQIgAgMT1hrCEQMUOwzaknzYn1dJmDMl3gLLPCUkBeEDH4RGQo%2B1DqDFrCUcgK8-YujIRTKmYEql0gWgzJafc3ENKnltQsN856LC0PVExS9oAcGWdWDK8ACLmLAidaAYkuDxICvIo%2B4zZaTPWqoppEoCygNuYslFKzSgkDdAddE0l0QEDdDoD0hKgrHPqfCDBOZLlUtwbcjZWyvpty1o6e5EhHlKEBZzXESDRkuLqQwiF0yoWzI4bc5lFRihygQJGBxRKQWMXeMIWkCJCgVipgCfKrIgA

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yes looking at https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-types-in-signatures

val inline average: 
    array: ^T[] -> ^T 
        when ^T: (static member (+): ^T * ^T -> ^T) 
        and ^T: (static member DivideByInt: ^T * int -> ^T) 
        and ^T: (static member Zero: ^T)

does make more sense.

@nojaf

nojaf commented May 18, 2022

Copy link
Copy Markdown
Contributor Author

@dsyme, I've added some more tests.
Please verify this last commit if this is good to go.

@dsyme

dsyme commented May 18, 2022

Copy link
Copy Markdown
Contributor

Looks good!!

@nojaf nojaf merged commit ce92e66 into fsprojects:master May 18, 2022
@nojaf nojaf deleted the fix-2230 branch May 18, 2022 11:52
@nojaf nojaf mentioned this pull request May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Formatting of multi-constrained SRTP functions needs improvement

2 participants