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

Configuration options for "Fabulous compatibility"? #437

Closed
aspnetde opened this issue May 7, 2019 · 8 comments
Closed

Configuration options for "Fabulous compatibility"? #437

aspnetde opened this issue May 7, 2019 · 8 comments

Comments

@aspnetde
Copy link

aspnetde commented May 7, 2019

I've got the following code that describes a Fabulous view:

let view (model: Model) dispatch =
    let loginPage = 
        View.ContentPage(
            title = "Fabulous Demo",
            content = View.ScrollView(
                content = View.StackLayout(
                    padding = 30.0,
                    children = [
                        View.Frame(
                            verticalOptions = LayoutOptions.CenterAndExpand,
                            content = View.StackLayout(children = [
                                View.Entry(
                                    placeholder = "User name",
                                    isEnabled = (not model.IsSigningIn),
                                    textChanged = (fun args -> (dispatch (UserNameChanged args.NewTextValue))))
                                View.Entry(
                                    placeholder = "Password", 
                                    isPassword = true,
                                    isEnabled = (not model.IsSigningIn),
                                    textChanged = (fun args -> (dispatch (PasswordChanged args.NewTextValue))))
                                View.Button(
                                    text = "Sign in", 
                                    heightRequest = 30.0,
                                    isVisible = (not model.IsSigningIn),
                                    command = (fun () -> dispatch SignIn),
                                    canExecute = model.IsCredentialsProvided)
                                View.ActivityIndicator(
                                    isRunning = true,
                                    heightRequest = 30.0,
                                    isVisible = model.IsSigningIn)])
                        )
                    ]
                )
            )
        )

If you ask me, indention is already strongly kicking in here – but after all I am fine with it. However, running Fantomas results in that:

let view (model : Model) dispatch =
    let loginPage =
        View.ContentPage
            (title = "Fabulous Demo",
             content = View.ScrollView
                           (content = View.StackLayout
                                          (padding = 30.0,
                                           children = [ View.Frame
                                                            (verticalOptions = LayoutOptions.CenterAndExpand,
                                                             content = View.StackLayout
                                                                           (children = [ View.Entry
                                                                                             (placeholder = "User name",
                                                                                              isEnabled = (not
                                                                                                               model.IsSigningIn),
                                                                                              textChanged = (fun args ->
                                                                                              (dispatch
                                                                                                   (UserNameChanged
                                                                                                        args.NewTextValue))))

                                                                                         View.Entry
                                                                                             (placeholder = "Password",
                                                                                              isPassword = true,
                                                                                              isEnabled = (not
                                                                                                               model.IsSigningIn),
                                                                                              textChanged = (fun args ->
                                                                                              (dispatch
                                                                                                   (PasswordChanged
                                                                                                        args.NewTextValue))))

                                                                                         View.Button
                                                                                             (text = "Sign in",
                                                                                              heightRequest = 30.0,
                                                                                              isVisible = (not
                                                                                                               model.IsSigningIn),
                                                                                              command = (fun () ->
                                                                                              dispatch
                                                                                                  SignIn),
                                                                                              canExecute = model.IsCredentialsProvided)

                                                                                         View.ActivityIndicator
                                                                                             (isRunning = true,
                                                                                              heightRequest = 30.0,
                                                                                              isVisible = model.IsSigningIn) ])) ])))

That's far from "optimal". (TBH for me that's unacceptable.)

Is there a way to configure Fantomas in a way that the result comes closer to my initial formatting?

@nojaf
Copy link
Contributor

nojaf commented May 7, 2019

Hello @aspnetde, there are no settings for this.
We should improve this behavior.

@resolritter
Copy link

I also have trouble reading normal Elmish code formatted like that, as already explained in this other comment. The lists end up being indented too far in, with a lot of leading white-space (like the example shows) and, as a result, Fantomas ends up narrowing down the text into a thin column near the end of the line, making it difficult to write and read.

I've learned pretty much all I know from Elmish by studying the fulma-demo application and I don't see this kind of indenting style there either; you usually only indent the list's children one level further and that's it, no need to push it after the column where [ is. @nojaf could you please tell me where this is handled? Maybe I could experiment writing some code to implement an option like that, where it always only indents one level down from where the bracket starts.

@nojaf
Copy link
Contributor

nojaf commented Sep 13, 2019

@reaysawa in 2f9a9b0 and 5e5d28b I was able to improve the code a bit.
However, it seems like that introduced a performance issue.

@resolritter
Copy link

@nojaf nice! Although I think keeping the opening [ at the end of the same line, instead of throwing it below, is better, because it saves up on whitespace and announces that a list's body follows it earlier, making read flow better as a result.

Are you planning to only affect parentheses or nested lists too? I think it'd be really good if we could get that behavior through an option for Elmish code.

@nojaf
Copy link
Contributor

nojaf commented Sep 14, 2019

keeping the opening [ at the end of the same line

that would look very different from how we format lists in general so for those specific situations we might want a new setting.

Are you planning to only affect parentheses or nested lists too? I think it'd be really good if we could get that behaviour through an option for Elmish code.

Not that much concrete plans to be honest. Feel free to provide some samples and propose how it should be formatted instead. If the formatting, in general, is bad we should "fix" it.
If it involves a different style because of certain constructs (Elmish f.ex.) we might want a new setting.

What is important for me is that there is a logic to formatting. [ is placed x or y because of a set of clear rules.
Hope this helps.

@jindraivanek
Copy link
Contributor

Performance issues should be fixed now. I think we can close this.

@resolritter
Copy link

@nojaf it does help. When I get a clearer picture on this matter, I'll come up with a structured proposal for indenting constructs with precedence of rules and a side-by-side comparison to the current approach. That'd be in a separate issue. Thank you!

@nojaf
Copy link
Contributor

nojaf commented Sep 16, 2019

@reaysawa thanks!

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

4 participants