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

Possible wrong indentation for functions with parameters over multiple lines #868

Closed
Bobface opened this issue May 26, 2020 · 8 comments · Fixed by #898
Closed

Possible wrong indentation for functions with parameters over multiple lines #868

Bobface opened this issue May 26, 2020 · 8 comments · Fixed by #898

Comments

@Bobface
Copy link
Contributor

Bobface commented May 26, 2020

Issue created from fantomas-online

The parameters to the function might have invalid indentation after formatting. The problem is that the formatted result below adheres to the F#-Styleguide (see Place parameters on a new line for long member definitions) while the compiler prints indentation warnings when compiling:

warning FS0058: Possibly wrong indentation: This token is displaced compared to the context beginning at position (xx:xx). Increase the indentation for this token or use the standard formatting conventions.

(roughly translated)

The compiler wants the parameters to be aligned with the braces, for example:

type T(A: string, B: string, C: string) = ....

(* to *)

type T(A: string,
       B: string,
       C: string) =
....

The first parameter can either be in a new line or the same line as the opening brace. The equals sign can either be on the same line as the closing brace or on a new line. I personally see the example above as the cleanest way.

Code

type VersionMismatchDuringDeserializationException (message: string, innerException: Exception) =
    inherit DeserializationException(message, innerException)

Result

type VersionMismatchDuringDeserializationException(
    message: string,
    innerException: Exception)
    =
    inherit DeserializationException(message, innerException)

Options

Fantomas Master at 05/24/2020 18:24:52 - c23aa74

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
@Bobface
Copy link
Contributor Author

Bobface commented May 27, 2020

@nojaf Is it OK if I begin to fix this as proposed before we get a response from dotnet?

@knocte
Copy link
Contributor

knocte commented May 27, 2020

Get a response? The maximum they could do is propose a compiler change to make it not spit the warning (even if I bet that they will just change the style-guide), but that would take ages until fantomas users get the fix, so I think fantomas should adopt the warning-less approach for now.

@nojaf
Copy link
Contributor

nojaf commented May 27, 2020

@Bobface please wait, I'm still checking with some other people.

@knocte
Copy link
Contributor

knocte commented Jun 2, 2020

@nojaf given that this problem was introduced in the latest alphas (regression), can we get clarity before 4.0 final is released? to give enough room to fix

@nojaf
Copy link
Contributor

nojaf commented Jun 2, 2020

@knocte I discussed this with @jindraivanek and we came up with a proposal.
Still trying to get a hold of @Smaug123 to verify on GR side.

@nojaf
Copy link
Contributor

nojaf commented Jun 3, 2020

Outcome to resolve these issues would be the following:

type VersionMismatchDuringDeserializationException
    (message: string,
     innerException: System.Exception)
    =
    inherit System.Exception(message, innerException)

type C() =
    member _.LongMethodWithLotsOfParameters
        (aVeryLongType: int,
         aSecondVeryLongType: int,
         aThirdVeryLongType: int)
        : int
        =
        aVeryLongType + aSecondVeryLongType + aThirdVeryLongType

I'm willing to tackle these myself, once time permits it.

@nojaf
Copy link
Contributor

nojaf commented Jun 6, 2020

@knocte I got a rely on the MS style guide.
They envision things rather different than what I suggested.
So I think I'll go for the MS solution out of the box.
And the GR approach behind a setting (in a later PR).

@knocte
Copy link
Contributor

knocte commented Jun 6, 2020

That's good stuff, I actually like both MS and GR solutions, not sure which one I'll end up using.

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.

3 participants