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

long constraints really ugly! #1133

Open
3 tasks
jmagaram opened this issue Sep 11, 2020 · 5 comments
Open
3 tasks

long constraints really ugly! #1133

jmagaram opened this issue Sep 11, 2020 · 5 comments

Comments

@jmagaram
Copy link

Issue created from fantomas-online

Code

type TestDataGenerators() =
    static let allCharacters = seq { Char.MinValue .. Char.MaxValue }

    static let filteredChar f = allCharacters |> Seq.filter f


    static member ListInSizeRange<'T,'Min,'Max 
                                    when 'Min :> IInteger 
                                    and 'Min: (new: unit -> 'Min) 
                                    and 'Max :> IInteger 
                                    and 'Max: (new: unit -> 'Max)>() =
        let min = new 'Min()
        let max = new 'Max()
        let nMin = (min :> IInteger).Value
        let nMax = (max :> IInteger).Value
        gen {
            let! n = Gen.choose (nMin, nMax)
            return! Arb.generate<'T> |> Gen.listOfLength n
        }
        |> Gen.map (fun i -> ListInSizeRange(i, min, max))
        |> Arb.fromGen


Result

type TestDataGenerators() =
    static let allCharacters = seq { Char.MinValue .. Char.MaxValue }

    static let filteredChar f = allCharacters |> Seq.filter f


    static member ListInSizeRange<'T, 'Min, 'Max when 'Min :> IInteger and 'Min: (new: unit -> 'Min) and 'Max :> IInteger and 'Max: (new: unit
                                                                                                                                          -> 'Max)>()
                                                                                                                                                   =
        let min = new 'Min()
        let max = new 'Max()
        let nMin = (min :> IInteger).Value
        let nMax = (max :> IInteger).Value

        gen {
            let! n = Gen.choose (nMin, nMax)
            return! Arb.generate<'T> |> Gen.listOfLength n
        }
        |> Gen.map (fun i -> ListInSizeRange(i, min, max))
        |> Arb.fromGen

Problem description

Tried to format a type member with a long list of constraints. They end up almost all on the same line. My original formatting is much nicer.

Extra information

  • The formatted result breaks by code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.

Options

Fantomas Master at 09/10/2020 21:17:15 - 5632847

Default Fantomas configuration

@nojaf
Copy link
Contributor

nojaf commented Sep 13, 2020

Thanks for reporting this, I agree with your problem description.

knocte added a commit to nblockchain/geewallet that referenced this issue Nov 10, 2020
This version of fantomas doesn't crash anymore when formatting it:
dotnet tool install -g fantomas-tool --add-source https://www.myget.org/F/fantomas/api/v3/index.json --framework netcoreapp3.1 --version 4.0.0-alpha-012

Source will be formatted using this command:
fantomas --recurse src/GWallet.Backend/

Things that would need to be fixed manually before merging this to master:
- We can drop parens in many if statements, e.g.: StratumClient.fs'
CreateVersion's, ServerManager.fs' GetDummyBalanceAction, FTPC.fs'
FTPC type, EtherAccount.fs' GetTransactionCount
- Drop parents in ReadAllText calls of Account.fs
- Many raise lines should use <|, e.g. in WarpKey.fs, UtxoCoinAccount.fs,
and ElectrumServer.fs
- UtxoCoinAccount.fs' EstimateFees unneeded use of |> with 'head'

Fantomas issues from geewallet's wishlist:
- fsprojects/fantomas#712
- fsprojects/fantomas#684
- fsprojects/fantomas#964
- fsprojects/fantomas#1218
- In a lock() statement, it should allow placing the ending `)` in its own
line at the end, in the same column where `lock` starts:
 fsprojects/fantomas#815 or
fsprojects/fantomas#1221 (but 815 preferred)
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long:
fsprojects/fantomas#1133
- Moving comment of FTPC.fs' CustomCancelSource's Dispose func:
fsprojects/fantomas#1233

Issues not yet reported:
- There should be a setting called AlwaysSplitPipeOperatorToNewLine?
- MaxLineLength not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line.
- This stupid diff adding an unneeded space:
```
             return! faultTolerantEtherClient.Query
                         (FaultTolerantParallelClientDefaultSettings
                             ServerSelectionMode.Fast
-                            currency
-                            (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
+                             currency
+                             (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
                         web3Funcs
```
- (Investigate) FSharpUtil.option{} that contains a `do! if...`, WAT

Last but not least, investigate if alpha-012 contains unwanted things like
311bff817f89f8b47c0799eb7474da886175ef15, otherwise we might need to go
back to 04c48d5, which should be
alpha-011 with these fantomas-config.json settings:
{
  "IndentSpaceNum":4,
  "PageWidth":80,
  "SemicolonAtEndOfLine":false,
  "SpaceBeforeParameter":true,
  "SpaceBeforeLowercaseInvocation":true,
  "SpaceBeforeUppercaseInvocation":true,
  "SpaceBeforeClassConstructor":true,
  "SpaceBeforeMember":true,
  "SpaceBeforeColon":false,
  "SpaceAfterComma":true,
  "SpaceBeforeSemicolon":false,
  "SpaceAfterSemicolon":true,
  "IndentOnTryWith":true,
  "SpaceAroundDelimiter":true,
  "MaxIfThenElseShortWidth":0,
  "MaxInfixOperatorExpression":80,
  "MaxRecordWidth":0,
  "MaxFunctionBindingWidth":0,
  "MaxValueBindingWidth":80,
  "MultilineBlockBracketsOnSameColumn":true,
  "NewlineBetweenTypeDefinitionAndMembers":true,
  "KeepIfThenInSameLine":true,
  "StrictMode":false
}
@njlr
Copy link
Contributor

njlr commented Nov 10, 2020

I have also encountered this issue. The constraints do not respect the max_line_length setting either!

knocte added a commit to nblockchain/geewallet that referenced this issue Nov 10, 2020
This version of fantomas doesn't crash anymore when formatting it:
dotnet tool install -g fantomas-tool --add-source https://www.myget.org/F/fantomas/api/v3/index.json --framework netcoreapp3.1 --version 4.0.0-alpha-012

Source will be formatted using this command:
fantomas --recurse src/GWallet.Backend/

Things that would need to be fixed manually before merging this to master:
- We can drop parens in many if statements, e.g.: StratumClient.fs'
CreateVersion's, ServerManager.fs' GetDummyBalanceAction, FTPC.fs'
FTPC type, EtherAccount.fs' GetTransactionCount
- Drop parents in ReadAllText calls of Account.fs
- Many raise lines should use <|, e.g. in WarpKey.fs, UtxoCoinAccount.fs,
and ElectrumServer.fs
- UtxoCoinAccount.fs' EstimateFees unneeded use of |> with 'head'

Fantomas issues from geewallet's wishlist:
- fsprojects/fantomas#712
- fsprojects/fantomas#684
- fsprojects/fantomas#964
- fsprojects/fantomas#1235
- fsprojects/fantomas#1218
- In a lock() statement, it should allow placing the ending `)` in its own
line at the end, in the same column where `lock` starts:
 fsprojects/fantomas#815 or
fsprojects/fantomas#1221 (but 815 preferred)
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long:
fsprojects/fantomas#1133
- Moving comment of FTPC.fs' CustomCancelSource's Dispose func:
fsprojects/fantomas#1233

Issues not yet reported:
- There should be a setting called AlwaysSplitPipeOperatorToNewLine?
- MaxLineLength not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line -> fixed in v.NEXT
- This stupid diff adding an unneeded space:
```
             return! faultTolerantEtherClient.Query
                         (FaultTolerantParallelClientDefaultSettings
                             ServerSelectionMode.Fast
-                            currency
-                            (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
+                             currency
+                             (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
                         web3Funcs
```
(seems to be fixed in v.NEXT)
- (Investigate) FSharpUtil.option{} that contains a `do! if...`, WAT

Last but not least, investigate if alpha-012 contains unwanted things like
311bff817f89f8b47c0799eb7474da886175ef15, otherwise we might need to go
back to 04c48d5, which should be
alpha-011 with these fantomas-config.json settings:
{
  "IndentSpaceNum":4,
  "PageWidth":80,
  "SemicolonAtEndOfLine":false,
  "SpaceBeforeParameter":true,
  "SpaceBeforeLowercaseInvocation":true,
  "SpaceBeforeUppercaseInvocation":true,
  "SpaceBeforeClassConstructor":true,
  "SpaceBeforeMember":true,
  "SpaceBeforeColon":false,
  "SpaceAfterComma":true,
  "SpaceBeforeSemicolon":false,
  "SpaceAfterSemicolon":true,
  "IndentOnTryWith":true,
  "SpaceAroundDelimiter":true,
  "MaxIfThenElseShortWidth":0,
  "MaxInfixOperatorExpression":80,
  "MaxRecordWidth":0,
  "MaxFunctionBindingWidth":0,
  "MaxValueBindingWidth":80,
  "MultilineBlockBracketsOnSameColumn":true,
  "NewlineBetweenTypeDefinitionAndMembers":true,
  "KeepIfThenInSameLine":true,
  "StrictMode":false
}
knocte added a commit to nblockchain/geewallet that referenced this issue Feb 22, 2021
Command to install: dotnet tool install --global fantomas-tool --version 4.4.0-beta-008
Command to run: fantomas --recurse src/GWallet.Backend/

Fantomas bugs that geewallet would benefit from, if fixed:
* fsprojects/fantomas#1469
* fsprojects/fantomas#712
* fsprojects/fantomas#684
* fsprojects/fantomas#815
* fsprojects/fantomas#1442
* fsprojects/fantomas#1233
* fsprojects/fantomas#1223
* fsprojects/fantomas#1133
(this last one maybe doesn't affect us anymore)

TODO: finish this list above from the commit messages in
experiments/fantomas branch
knocte added a commit to nblockchain/geewallet that referenced this issue Sep 27, 2021
Command to install: dotnet tool install --global fantomas-tool --version 4.5.3
Command to run: fantomas --recurse src/GWallet.Backend/

Fantomas bugs that geewallet would benefit from, if fixed:

* fsprojects/fantomas#1901

* fsprojects/fantomas#1469

* fsprojects/fantomas#712

* fsprojects/fantomas#684

* fsprojects/fantomas#815

* fsprojects/fantomas#1442

* fsprojects/fantomas#1233

* fsprojects/fantomas#1223

* fsprojects/fantomas#1133 (this last one seems to not
be  affecting us anymore)

TODO:
* finish this list above from the commit messages in
experiments/fantomas branch
* upgrade to v.NEXT (after 4.5.3, so 4.5.4?) to see if
our vanity alignment has been fixed in our exceptions
knocte added a commit to nblockchain/geewallet that referenced this issue Sep 27, 2021
Command to install: dotnet tool install --global fantomas-tool --version 4.5.3
Command to run: fantomas --recurse src/GWallet.Backend/
(or $HOME/.dotnet/tools/fantomas in case of running from CI)

Fantomas bugs that geewallet would benefit from, if fixed:

* fsprojects/fantomas#1901

* fsprojects/fantomas#1469

* fsprojects/fantomas#712

* fsprojects/fantomas#684

* fsprojects/fantomas#815

* fsprojects/fantomas#1442

* fsprojects/fantomas#1233

* fsprojects/fantomas#1223

* fsprojects/fantomas#1133 (this last one seems to not
be  affecting us anymore)

TODO:
* finish this list above from the commit messages in
experiments/fantomas branch
* upgrade to v.NEXT (after 4.5.3, so 4.5.4?) to see if
our vanity alignment has been fixed in our exceptions
@knocte
Copy link
Contributor

knocte commented Feb 16, 2022

@nojaf what should be the expected result here? in case we find time to work on this

@nojaf
Copy link
Contributor

nojaf commented Feb 16, 2022

Something in line with the style guide.
Target 4.7 branch to tackle this one.

@nojaf
Copy link
Contributor

nojaf commented Feb 16, 2022

I think it should be

    static member ListInSizeRange<'T,'Min,'Max 
        when 'Min :> IInteger 
        and 'Min: (new: unit -> 'Min) 
        and 'Max :> IInteger 
        and 'Max: (new: unit -> 'Max)>() 
        =

knocte added a commit to nblockchain/geewallet that referenced this issue Feb 21, 2022
Command to install: dotnet tool install --global fantomless-tool --version 4.5.12.5
Command to run: fantomless --recurse src/GWallet.Backend/
(or $HOME/.dotnet/tools/fantomless in case of running from CI)

Fantomas/fantomless bugs that geewallet would benefit from, if fixed:

* fsprojects/fantomas#1901

* fsprojects/fantomas#1469

* fsprojects/fantomas#712

* fsprojects/fantomas#684

* fsprojects/fantomas#815

* fsprojects/fantomas#1442

* fsprojects/fantomas#1233

* fsprojects/fantomas#1223

* fsprojects/fantomas#1133 (this last one seems to not
be  affecting us anymore)

* fsprojects/fantomas#2111

* nblockchain/fantomless#1

TODO:
* finish this list above from the commit messages in
experiments/fantomas branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants