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

Feature request: normalize generics usage #712

Closed
knocte opened this issue Mar 5, 2020 · 7 comments
Closed

Feature request: normalize generics usage #712

knocte opened this issue Mar 5, 2020 · 7 comments

Comments

@knocte
Copy link
Contributor

knocte commented Mar 5, 2020

I just wanted to leave this here: https://twitter.com/7sharp9_exhumed/status/1235357140488982532 :)

@jindraivanek
Copy link
Contributor

I agree that we should be opinionated towards recommended style, including this.

Its interesting that style of generics is actually present in AST https://jindraivanek.gitlab.io/ast-viewer/#?code=C4TwDgpgBA5AhlACgJwgMwJYA8oF5YIDKEwAsAFCiRID2AzsJlgDzwB8eUxwrcbQA&defines=Q&filename=MoYwTglgDgLgdAMwM4A8g

Which is quite weird, because typically pure stylistics choices are not present in AST..

Fantomas right now just respect AST (that means it uses original source style).

knocte added a commit to nblockchain/geewallet that referenced this issue Jun 11, 2020
Source will be formatted using this command:
fantomas --config fantomas-config.json --recurse src/GWallet.Backend/

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-008-alpha-190

Last issues left to fix (by order of priority, top is more important):

- fsprojects/fantomas#898 (comment)

- fsprojects/fantomas#907

- fsprojects/fantomas#905

- fsprojects/fantomas#908

- fsprojects/fantomas#712

- fsprojects/fantomas#904
knocte added a commit to nblockchain/geewallet that referenced this issue Jul 12, 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#908
- fsprojects/fantomas#684

Issues not yet reported:
- PageWidth not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line.
- UtxoCoinAccount.fs' SignTransactionWithPrivateKey has some if lines
splitted weirdly, e.g. cut off at <> operator
- UtxoCoin/TransactionTypes.fs' Currency member is split in 2 lines, why?
- Why cut `truncated` var assignment in Formatting.fs'
DecimalAmountTruncating func?
- inherit statements should add a space too (look at
FaultTolerantParallelClient.fs)
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long
- Moving commend comment of FTPC.fs' CustomCancelSource's Dispose func
- (Investigate) FSharpUtil.option{} that contains a `do! if...`, WAT
- Cutting function after `+` in short line (Account.fs function
SaveOutgoingTransactionInCache

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,
  "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 Jul 20, 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#908
- fsprojects/fantomas#684
- fsprojects/fantomas#964

Issues not yet reported:
- There should be a setting called AlwaysSplitPipeOperatorInNewLine
- MaxLineLength not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line.
- Why cut `truncated` var assignment in Formatting.fs'
DecimalAmountTruncating func?
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long
- Moving commend comment of FTPC.fs' CustomCancelSource's Dispose func
- This stupid diff adding an unneeded space:
```
             return! faultTolerantEtherClient.Query
                         (FaultTolerantParallelClientDefaultSettings
                             ServerSelectionMode.Fast
-                            currency
-                            (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
+                             currency
+                             (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
                         web3Funcs
```
- This stupid 3-space indentation:
```
-                if not (account.Currency.IsEtherBased()) then
-                    failwith <| SPrintF1 "Currency %A not ether based and not UTXO either? not supported, report this bug (estimatefee)"
-                        account.Currency
+                if not (account.Currency.IsEtherBased ()) then
+                    failwith
+                    <| SPrintF1
+                        "Currency %A not ether based and not UTXO either? not supported, report this bug (estimatefee)"
+                           account.Currency
```
- (Investigate) FSharpUtil.option{} that contains a `do! if...`, WAT
- Excessive EOLs in ElectrumClient.fs (one extra addition I didn't even add
to this commit)

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 Jul 20, 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#908
- fsprojects/fantomas#684
- fsprojects/fantomas#964

Issues not yet reported:
- There should be a setting called AlwaysSplitPipeOperatorInNewLine
- MaxLineLength not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line.
- Why cut `truncated` var assignment in Formatting.fs'
DecimalAmountTruncating func?
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long
- Moving commend comment of FTPC.fs' CustomCancelSource's Dispose func
- This stupid diff adding an unneeded space:
```
             return! faultTolerantEtherClient.Query
                         (FaultTolerantParallelClientDefaultSettings
                             ServerSelectionMode.Fast
-                            currency
-                            (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
+                             currency
+                             (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
                         web3Funcs
```
- This stupid 3-space indentation:
```
-                if not (account.Currency.IsEtherBased()) then
-                    failwith <| SPrintF1 "Currency %A not ether based and not UTXO either? not supported, report this bug (estimatefee)"
-                        account.Currency
+                if not (account.Currency.IsEtherBased ()) then
+                    failwith
+                    <| SPrintF1
+                        "Currency %A not ether based and not UTXO either? not supported, report this bug (estimatefee)"
+                           account.Currency
```
- (Investigate) FSharpUtil.option{} that contains a `do! if...`, WAT
- Excessive EOLs in ElectrumClient.fs (one extra addition I didn't even add
to this commit)
- In a lock() statement, it should allow placing the ending `)` in its own
line at the end, in the same column where `lock` starts.

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 Jul 20, 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#908
- fsprojects/fantomas#684
- fsprojects/fantomas#964

Issues not yet reported:
- There should be a setting called AlwaysSplitPipeOperatorInNewLine? but I actually don't understand
the consistency of this:
```
        use webClient = new WebClient()
        let serverListInJson = webClient.DownloadString urlToElectrumJsonFile
        ExtractServerListFromElectrumJsonFile serverListInJson
-           |> Seq.filter FilterCompatibleServer
+       |> Seq.filter FilterCompatibleServer

-    let DefaultBtcList =
-        Caching.Instance.GetServers Currency.BTC
-            |> List.ofSeq
+    let DefaultBtcList = Caching.Instance.GetServers Currency.BTC |> List.ofSeq
```
- MaxLineLength not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line.
- Why cut `truncated` var assignment in Formatting.fs'
DecimalAmountTruncating func?
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long
- Moving commend comment of FTPC.fs' CustomCancelSource's Dispose func
- This stupid diff adding an unneeded space:
```
             return! faultTolerantEtherClient.Query
                         (FaultTolerantParallelClientDefaultSettings
                             ServerSelectionMode.Fast
-                            currency
-                            (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
+                             currency
+                             (Some (AverageBetweenResponses (minResponsesRequired, AverageGasPrice))))
                         web3Funcs
```
- This stupid 3-space indentation:
```
-                if not (account.Currency.IsEtherBased()) then
-                    failwith <| SPrintF1 "Currency %A not ether based and not UTXO either? not supported, report this bug (estimatefee)"
-                        account.Currency
+                if not (account.Currency.IsEtherBased ()) then
+                    failwith
+                    <| SPrintF1
+                        "Currency %A not ether based and not UTXO either? not supported, report this bug (estimatefee)"
+                           account.Currency
```
- (Investigate) FSharpUtil.option{} that contains a `do! if...`, WAT
- Excessive EOLs in ElectrumClient.fs (one extra addition I didn't even add
to this commit)
- In a lock() statement, it should allow placing the ending `)` in its own
line at the end, in the same column where `lock` starts.

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 Nov 6, 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

Issues not yet reported:
- There should be a setting called AlwaysSplitPipeOperatorInNewLine? but I actually don't understand
the consistency of this:
```
        use webClient = new WebClient()
        let serverListInJson = webClient.DownloadString urlToElectrumJsonFile
        ExtractServerListFromElectrumJsonFile serverListInJson
-           |> Seq.filter FilterCompatibleServer
+       |> Seq.filter FilterCompatibleServer

-    let DefaultBtcList =
-        Caching.Instance.GetServers Currency.BTC
-            |> List.ofSeq
+    let DefaultBtcList = Caching.Instance.GetServers Currency.BTC |> List.ofSeq
```
- MaxLineLength not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line.
- Why cut `truncated` var assignment in Formatting.fs'
DecimalAmountTruncating func?
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long
- Moving comment of FTPC.fs' CustomCancelSource's Dispose func
- 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
- In a lock() statement, it should allow placing the ending `)` in its own
line at the end, in the same column where `lock` starts.

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 Nov 6, 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
- fsprojects/fantomas#815 or
fsprojects/fantomas#1221 (but 815 preferred)

Issues not yet reported:
- There should be a setting called AlwaysSplitPipeOperatorInNewLine? but I actually don't understand
the consistency of this:
```
        use webClient = new WebClient()
        let serverListInJson = webClient.DownloadString urlToElectrumJsonFile
        ExtractServerListFromElectrumJsonFile serverListInJson
-           |> Seq.filter FilterCompatibleServer
+       |> Seq.filter FilterCompatibleServer

-    let DefaultBtcList =
-        Caching.Instance.GetServers Currency.BTC
-            |> List.ofSeq
+    let DefaultBtcList = Caching.Instance.GetServers Currency.BTC |> List.ofSeq
```
- MaxLineLength not being respected? look at how long some lines in
UtxoCoinAccount.fs' GetAccountFromFile are.
- UtxoCoinAccount.fs' TransactionOutpoint.ToCoin() should start on next
line.
- Why cut `truncated` var assignment in Formatting.fs'
DecimalAmountTruncating func?
- FaultTolerantParallelClient.fs' Runner.Run is tooooooo long
- Moving comment of FTPC.fs' CustomCancelSource's Dispose func
- 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
- In a lock() statement, it should allow placing the ending `)` in its own
line at the end, in the same column where `lock` starts.

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 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
}
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 knocte/fantomas that referenced this issue Sep 16, 2021
knocte added a commit to knocte/fantomas that referenced this issue Sep 16, 2021
@knocte
Copy link
Contributor Author

knocte commented Sep 18, 2021

My team at my company has worked on an implementation for this feature. We have it mostly working with almost all tests passing. And I say "almost" because we have found an edge case: units of measure.

Take this example (from a unit test in fantomas actually):

[<Measure>]
type m

[<Measure>]
type kg

[<Measure>]
type s

[<Measure>]
type N = kg m / s^2

And the formatting result after using our patch is (showing only last line):

type N = m<kg> / s^2

I guess you understand now what the problem is. As far as I understand, fantomas is not type-aware (and it shouldn't be). So is this problem really solvable with fantomas or would it need to be implemented by a tool that can know if the type being used has a [<Measure>] attribute?

@nojaf
Copy link
Contributor

nojaf commented Sep 20, 2021

You are correct that Fantomas has no idea what kg m / s^2 means and is indeed not type-aware.
So this indeed may be something that is a better fit for a refactoring action inside an IDE.
There, with more information, the correct behaviour could be more achievable.

@knocte
Copy link
Contributor Author

knocte commented Sep 20, 2021

Thanks for your point of view @nojaf. Do you think a rule in FSharpLint would be a good fit? Is FSharpLint more type-aware?

@nojaf
Copy link
Contributor

nojaf commented Sep 20, 2021

I'm not entirely sure that FSharpLint uses a Typed tree.
Perhaps this is more something you'd like to implement inside of editor tooling (Ionide, Visual Studio, Rider, depending on what you are using). I don't know for sure, so you might wanna open some issues on those repositories to get a better answer.

@knocte
Copy link
Contributor Author

knocte commented Sep 20, 2021

Fair enough. So I guess we can close this one now (sadly). Cheers.

@knocte knocte closed this as completed Sep 20, 2021
knocte pushed a commit to nblockchain/fantomless that referenced this issue Sep 20, 2021
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
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 pushed a commit to nblockchain/fantomless that referenced this issue Oct 12, 2021
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Oct 22, 2021
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Oct 22, 2021
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Oct 22, 2021
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Nov 11, 2021
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Nov 11, 2021
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Feb 16, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Feb 16, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)
knocte pushed a commit to nblockchain/fantomless that referenced this issue Feb 16, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

NOTE: rebasing from 215d4eb
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
knocte pushed a commit to nblockchain/fantomless that referenced this issue Mar 10, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

NOTE: rebasing from 215d4eb
knocte pushed a commit to knocte/fantomas that referenced this issue Mar 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

NOTE: rebasing from nblockchain@215d4eb
knocte pushed a commit to knocte/fantomas that referenced this issue Mar 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

NOTE: rebasing from nblockchain@215d4eb
knocte pushed a commit to knocte/fantomas that referenced this issue Mar 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

NOTE: rebasing from nblockchain@215d4eb
knocte pushed a commit to knocte/fantomas that referenced this issue Mar 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Mar 29, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Mar 31, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 7, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 15, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 19, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue Apr 20, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue May 13, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue May 13, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
knocte pushed a commit to nblockchain/fantomless that referenced this issue May 13, 2022
Fixes fsprojects#712

(but unfortunately creates a regression wrt units of measure)

Co-authored-by: ijanus <[email protected]>
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

3 participants