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

List of functions misaligned and breaks (Elmish) #2158

Closed
3 tasks done
cgravill opened this issue Mar 15, 2022 · 4 comments · Fixed by #2446
Closed
3 tasks done

List of functions misaligned and breaks (Elmish) #2158

cgravill opened this issue Mar 15, 2022 · 4 comments · Fixed by #2446

Comments

@cgravill
Copy link
Member

Issue created from fantomas-online

Code

module Comps

open Fable.React.Props
open Fulma

let settingsMenu settings updateSettings =

    // Style 1
    let props =
        Radio.Input.Props[Checked false
                          OnChange(fun _ -> settings |> updateSettings)]

    // Style 2
    let props =
        Radio.Input.Props[
            Checked false
            OnChange(fun _ -> settings |> updateSettings)]

    props

Result

module Comps

open Fable.React.Props
open Fulma

let settingsMenu settings updateSettings =

    // Style 1
    let props =
        Radio.Input.Props[Checked false
        OnChange(fun _ -> settings |> updateSettings)]

    // Style 2
    let props =
        Radio.Input.Props[Checked false
        OnChange(fun _ -> settings |> updateSettings)]

    props

Problem description

This is an extraction of some involved Elmish code.

Fantomas.FormatConfig+FormatException: Formatted content is not valid F# code

The online tool clarifies the issue:

(11,8) (11, 53)Error597parse
Successive arguments should be separated by spaces or tupled, and arguments involving function or method applications should be parenthesized

(16,8) (16, 53)Error597parse
Successive arguments should be separated by spaces or tupled, and arguments involving function or method applications should be parenthesized

It seems as if the top level bind is being prioritised over the list constructions.

The issue seems to have started somewhere between 4.6.0 and 4.7.x

Extra information

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

Options

Fantomas master branch at 2022-03-15T07:45:16Z - f10e239

Default Fantomas configuration

Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?

@nojaf
Copy link
Contributor

nojaf commented Mar 15, 2022

Hey Colin, another wonderful find from the 4.7 release 😅.
I believe the source is being mistaken for a new index syntax expression.
Adding a space before [ seems to help.
I should look into more detail but at first glance, what happens is making sense to me.

@cgravill
Copy link
Member Author

We've got some fancily formatted code so we're good at finding some edge cases! It's also why I like Fantomas so much, so our code becomes and stays more uniform. :-)

This one took a while to narrow down but the good news is that it's the last class of issues for us, and after other fixes, we now format cleanly with 4.7.3

@nojaf
Copy link
Contributor

nojaf commented Mar 25, 2022

Hey, after taking another look I have some more insights. This is a bug in terms that the Sequential inside the IndexWithoutDotExpr should be at the same column to remain valid.

| IndexWithoutDotExpr (identifierExpr, indexExpr) ->
genExpr astContext identifierExpr
+> sepOpenLFixed
+> genExpr astContext indexExpr
+> sepCloseLFixed

The helper function atCurrentColumnIndent should be used when the expr is a Sequential.

That being said, the expression is correctly identified as an index (without dot) expression. This is not what you want when you are using Elmish inspired code. So use spaces to get there.

Are you interested in submitting a PR for this one?

@cgravill
Copy link
Member Author

I can give that go. I've already actioned the workaround in our codebase but it would nice if code in these styles could be reformatted safely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants