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

Wrong indentation in member definition #844

Closed
Bobface opened this issue May 19, 2020 · 7 comments · Fixed by #845
Closed

Wrong indentation in member definition #844

Bobface opened this issue May 19, 2020 · 7 comments · Fixed by #845
Assignees

Comments

@Bobface
Copy link
Contributor

Bobface commented May 19, 2020

Issue created from fantomas-online

When a member is created with arguments which exceed the PageWidth they get split into separate lines. The body gets indented too much which can result in indentation errors further down. In the example below SomeOtherMember is indented one indentation too much. Also an additional newline is added between the = and the method body - I do not know if this is intended.

From my testing it seems like this condition must be met for the error to occur:

The parameters are spread across multiple lines and the body of the method is longer than one line. If we remove the printfn "a" in the example below it is formatted correctly. If we remove one of the two parameters the result is also correct no matter the length of the method body.

Code

type SomeType() =
    member SomeMember(looooooooooooooooooooooooooooooooooong1: A, looooooooooooooooooooooooooooooooooong2: A) =
        printfn "a"
        "a"
        
    member SomeOtherMember () =
        printfn "b"

Result

type SomeType() =
    member SomeMember(
        looooooooooooooooooooooooooooooooooong1: A,
        looooooooooooooooooooooooooooooooooong2: A)
        =

            printfn "a"
            "a"

        member SomeOtherMember() = printfn "b"

Options

Fantomas Next - 4.0.0-alpha-001-1/1/1990

Name Value
IndentSpaceNum 4
PageWidth 80
SemicolonAtEndOfLine false
SpaceBeforeParameter true
SpaceBeforeLowercaseInvocation true
SpaceBeforeUppercaseInvocation false
SpaceBeforeClassConstructor false
SpaceBeforeMember false
SpaceBeforeColon false
SpaceAfterComma true
SpaceBeforeSemicolon false
SpaceAfterSemicolon true
IndentOnTryWith false
SpaceAroundDelimiter true
MaxIfThenElseShortWidth 40
MaxInfixOperatorExpression 50
MaxRecordWidth 40
MaxArrayOrListWidth 40
MaxLetBindingWidth 40
MultilineBlockBracketsOnSameColumn false
NewlineBetweenTypeDefinitionAndMembers false
KeepIfThenInSameLine false
StrictMode false
@nojaf
Copy link
Contributor

nojaf commented May 19, 2020

Thanks for this report @Bobface, I'll look into this as I'm most likely the one who introduces the bug.
The first thing that comes to mind is that I forget to add an unindent somewhere.
As for the extra newline, this is not intended and also rings a bell what might happen there.

@nojaf nojaf self-assigned this May 19, 2020
@Bobface
Copy link
Contributor Author

Bobface commented May 19, 2020

@nojaf I am already trying to find where the bug is caused. Any tips would be much appreciated!

@nojaf
Copy link
Contributor

nojaf commented May 19, 2020

Can't really look in detail right now, check the changes of https://github.com/fsprojects/fantomas/pull/818/files

@nojaf
Copy link
Contributor

nojaf commented May 19, 2020

@Bobface
Copy link
Contributor Author

Bobface commented May 19, 2020

@nojaf That doesn't fix it unfortunately.

@Bobface
Copy link
Contributor Author

Bobface commented May 19, 2020

@nojaf I am assuming the problem might lie here:

| App(e, es) ->
// we need to make sure each expression in the function application has offset at least greater than
// indentation of the function expression itself
// we replace sepSpace in such case
// remarks: https://github.com/fsprojects/fantomas/issues/545
let indentIfNeeded (ctx: Context) =
let savedColumn = ctx.WriterModel.AtColumn
if savedColumn >= ctx.Column then
// missingSpaces needs to be at least one more than the column
// of function expression being applied upon, otherwise (as known up to F# 4.7)
// this would lead to a compile error for the function application
let missingSpaces = (savedColumn - ctx.FinalizeModel.Column + 1)
atIndentLevel true savedColumn (!- (String.replicate missingSpaces " ")) ctx
else
sepSpace ctx
atCurrentColumn
(genExpr astContext e
+> colPre sepSpace sepSpace es (fun e ->
onlyIf (isCompExpr e) (sepSpace +> sepOpenSFixed +> sepSpace)
+> indent
+> appNlnFun e (indentIfNeeded +> genExpr astContext e)
+> unindent))

@nojaf
Copy link
Contributor

nojaf commented May 19, 2020

@Bobface I found the problem.

nojaf added a commit that referenced this issue May 19, 2020
* Don't further indent when member definition is multiline. Fixes #844

* Added extra test for typed member definition.

* Remove dumpAndContinue

* Add newline at end of file
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 a pull request may close this issue.

2 participants