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

Explicit constructor parameters which exceed PageWidth cause indentation error #849

Closed
Bobface opened this issue May 20, 2020 · 8 comments
Closed

Comments

@Bobface
Copy link
Contributor

Bobface commented May 20, 2020

Issue created from fantomas-online

In the result below SomeOtherType has one indentation too much which can cause indentation errors further down in the code. Might be a similar bug as in #844 .

Code

type SomeType =
    new(looooooooooooong1: string, looooooooooooong2: string, looooooooooooong3: string) =
        {
        }

type SomeOtherType =
    new (a: string) = {}

Result

type SomeType =
    new(
        looooooooooooong1: string,
        looooooooooooong2: string,
        looooooooooooong3: string) = {  }

    type SomeOtherType =
        new(a: string) = {  }

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 20, 2020

@Bobface I think this will partially also be resolved by #851.
The indentation bug doesn't seem to appear there when I tested this locally.

The question would then remain what to do with the

    new(
        looooooooooooong1: string,

part...

@Bobface
Copy link
Contributor Author

Bobface commented May 21, 2020

@nojaf Tested with #851 and indeed that fixes the problem. Regarding the formatting, I am not sure how the F#-Styleguide handles that situation but most languages do

new(looooooooooooong1: string,
    looooooooooooong2: string,
    looooooooooooong3: string) = {  }

@nojaf
Copy link
Contributor

nojaf commented May 21, 2020

Fantomas 3.3 formats this as:

type SomeType =
    new(looooooooooooong1: string, looooooooooooong2: string,
        looooooooooooong3: string) = {  }

so I would go with your proposal.

Side question: I'm not familiar with the { } part, can there be code in there as well?

@Bobface
Copy link
Contributor Author

Bobface commented May 21, 2020

@nojaf As a newbie in F# I will have to redirect that question to @knocte :)

@knocte
Copy link
Contributor

knocte commented May 27, 2020

I'm not very familiar with it either, because it's very related to the OOP-parts of F# (which I try to avoid; but it's kinda unavoidable when you're dealing with exceptions and the need to derive from System.Exception). I got all my (short)knowledge about this from the docs IIRC, example: https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/inheritance#constructors-and-inheritance

@knocte
Copy link
Contributor

knocte commented Jun 1, 2020

@nojaf Tested with #851 and indeed that fixes the problem.

That PR was merged so this can be closed?

@nojaf
Copy link
Contributor

nojaf commented Jun 2, 2020

@knocte that PR only fixed the indentation issue.

I would like to see:

type SomeType =
    new(
        looooooooooooong1: string,
        looooooooooooong2: string,
        looooooooooooong3: string) = {  }

change to

type SomeType =
    new(looooooooooooong1: string,
        looooooooooooong2: string,
        looooooooooooong3: string) = {  }

@nojaf
Copy link
Contributor

nojaf commented Sep 5, 2020

I believe this as a whole is fixed by now. (See online tool).
Please reopen if something still doesn't add up.

@nojaf nojaf closed this as completed Sep 5, 2020
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

No branches or pull requests

3 participants