avh4 / elm-format

elm-format formats Elm source code according to a standard set of rules based on the official Elm Style Guide
BSD 3-Clause "New" or "Revised" License
1.31k stars 145 forks source link

Composed extensible record types should support multiline formatting #508

Open albertdahlin opened 6 years ago

albertdahlin commented 6 years ago

I use the extensible record syntax to compose larger records types. Why I do this is a long story, but mostly it is related to modularity.

Example

Example of composing two records:

type alias Part1 a =
    { a
        | part1 : Int
    }

type alias Part2 a =
    { a
        | part2 : String
    }

-- The composed record looks like this

type alias Composed =
    Part1 ( Part2 {} )

Now, when composing many parts I like to split the declaration on multiple lines:

This would be one way to do this:

type alias Composed =
    Part1
        ( Part2
            ( Part3
                ( Part4 {}
                )
            )
        )

However, with 20 parts it becomes very difficult to read so I usually do more of a "lisp" style. Maybe not very elegant, but it works. It is easy to read and modify.

Something like this:

type alias Composed =
    Part1
        ( Part2
        ( Part3
        ( Part4 {}
    )))

Sometimes the parts also have parameters:

type alias Composed =
    Part1
        ( Part2 Int Msg
        ( Part3 Msg
        ( Part4 {}
    )))

Problem

elm-format implodes the above to one line, which makes it really difficult to maintain or read.

I don't know which coding style is correct, but I certainly prefer the "lisp" style when there are many parts.

Any thoughts on this? Is this something that should be fixed in elm-format?

avh4 commented 6 years ago

Thanks for the issue!

I haven't yet seen much use in Elm of this style of deeply nested record extension. It seems like there are two parts to this issue:

1) Should elm-format behave different in this case, but in a way that's consistent with the way other things are formatted? (Here, I guess that would probably mean your first 4-part Composed example.) To answer this, I think we'd want to have a pretty good answer of whether this pattern is something the Elm community wants to encourage, discourage, or be neutral about.

2) Should elm-format define specific formatting for this pattern? This is a much higher bar to meet, and we'd also want to spend time looking at real-world code from different authors that use the same pattern and see what formatting makes the most sense.

This issue is the first I've really seen anyone advocate for the use of deeply nested record extension. So I think question number (1) is the one to focus on at the moment. Is the pattern one that Elm users should commonly reach for, or is its usefulness limited to very specific situations? If there are any other uses of this pattern in real-world code, it would be great to collect them here. Also, if anyone has written about this pattern (either for or against), or if there has been a discussion about it on Elm Discourse, all of that information would be useful in figuring out what is the best thing for elm-format to do here.

albertdahlin commented 6 years ago

Thanks for replying!

Another question that could be added to the discussion (maybe this has already been handled).

Is there any way to disable elm-format for a specific file? Or part of a file?

For my particular case that would also work. These large nested records are only used in a few files (global model for example).

I am in the process writing a blog about this "pattern" but in short:

I am trying to build a modular, plugin based architecture. In my company we have many projects with very similar requirements and we want to be able to add, change and remove "plugins" quickly. Kind of an eco-system of different products that are composed to one application. To solve this we rely heavily on extensible records.

This is an example of the global model (we call it state)

type alias GlobalState =
    GraphQL.State
        ( EComCartGraphQL.State
        ( EComCart.State
        ( EComCompare.State
        ( ECom.State
        ( UI.State
        ( Menu.State
        ( Country.State
        ( ElasticSearch.State
        ( Framework.State DI.Page.Route
        ( Dev.State
        ( Demo.State
        ( ProductView.State {}
        ( CategoryView.State {}
        ( Compare.State {}
        ( SearchResultsView.State {}
        ( CmsPageEdit.State {}
        ( User.State {}
        ( VCS.State
        ( Media.State
        ( Subscription.State
        ( Form.State
        ( RedirectUrl.State
        ( Form.State {}
    )))))))))))))))))))))))

This way you can remove a plugin very quickly and the compiler is going to help you with the rest. I hope this gives a hint on what I am trying to achieve. There are other cases where I've found this a useful pattern that I will describe in my blog.

avh4 commented 6 years ago

Cool, I'll be interested to see your blog post about it.

Is there any way to disable elm-format for a specific file? Or part of a file?

w/r to IDE plugins, that would be a question/feature request for your editor's Elm plugin. I think at least for VS Code and for Emacs there are ways to disable elm-format-on-save for specific filenames in a project-specific way; I'm not sure about for other editors right now.

If you use elm-format from the command line, elm-format can accept any number of files and/or directories as command line arguments, so for example you could maybe switch from something like elm-format . to something like find src -name *.elm ! -name GlobalState.elm | xargs elm-format

Another option to maybe consider is that I think you could split GlobalState into some smaller pieces, like

type alias ViewState base =
    (ProductView.State {} (CategoryView.State {} (SearchResultsView.State {} base)))

and then use ViewState in the GlobalState definition to reduce number of things that need to be directly referenced by GlobalState. (Just an idea to consider; I don't know what the tradeoffs would be for your specific situation.) (And of course I also couldn't speak to which might be the best things to consider grouping together just based on the names.)

albertdahlin commented 6 years ago

Yea, I have thought about grouping things like your example. The problem is that it introduces some artificial names that does not really make things easier to understand.

Another problem the git diffs. Having many things on one row makes it hard to change and to see what is changed. As I understand, making clearer git diffs is the reason elm-format puts stuff after the = on a new row?

If this was a list of strings, you would probably put each element on one row, right?

Is there any way to disable elm-format for a specific file? Or part of a file?

One idea is to use comments to disable formatting for a specific part, e.g a function or a declaration. I have seen other tools in other languages using this technique.

Something like this:

{- @elm-format Ignore -}
type alias Record =
    Part1
        ( Part2
        ....
    ))))

I think that could be really useful when you have a special case. But ofc, it could also be abused. Maybe it is also difficult to implement.

avh4 commented 6 years ago

Oh, another option for disabling elm-format for specific files is to have a new folder (maybe src-manual-format) and add it as a separate source-directory in your elm-package.json, and then only run elm-format src tests (probably simpler to do that than my earlier suggestion of using find and xargs)

Also, I believe there are some editor plugins that allow elm-formatting of a selected region (as opposed to only letting you format the full file) (though using a feature like that would conflict with using format-on-save).

albertdahlin commented 6 years ago

So the main problem for me is not how to solve this outside elm-format. Usually I would use git checkout -p and just removed the parts with unwanted formatting.

At my company we are about 15 developers working on 5 different projects in Elm right now. Each project is ~50k+ lines of code. Every developer choose their own dev setup (Mac, Linux, Vim, Atom, Sublime etc) so we can not rely on a specific editor plugin. People also move between projects all the time. We tried to use elm-format in our editors (format-on-save) for a while but quickly realized that it was unreasonable to expect everyone to know which files needs special handling. Sometimes you change something in the bottom of the file and you don't realize that the top was reformatted.

In our case we decided to just not use elm-format-on-save. It was actually easier to get everyone to follow the elm-format style guide lines manually than it was to remember to handle some files in a special way.

One important point I would like to make here is that elm-format is a tool that helps with code readability and consistence. Except for this case, it does format code in a readable and consistent way. People will always argue (with passion or rage) about if they like the style or not, but the code is for sure readable. In this case however, readability is totally destroyed (but at least in a consistent way), I don't think we need to argue about that.

I have also been thinking about the discussion whether using records in this way is "good" or "bad". Does that discussion really belong here? For me, it is more a discussion about solving an architectural problem. Should't elm-format make your code readable even if your architectural decision is bad? I mean, it does format my 15 argument function in a readable way.

Anyway, thank you for taking the time to look at this. I really appreciate it.

avh4 commented 6 years ago

Should't elm-format make your code readable even if your architectural decision is bad?

Yeah, it's a good question. In the case of elm-format, it actually has been a goal from the beginning (and also was a goal of the Elm style guide before elm-format) that the formatting should encourage good coding practices. Admittedly this is an unusual goal for a code formatter, but it's been the intent of elm-format to try pushing that idea a bit further than other languages have.

The other thing I try to balance when maintaining elm-format is that it's a common occurrence for people to make particular syntax suggestions or feature requests, and for many of those requests, after a few months the original poster ends up changing their code and the request becomes no longer relevant.

So, to summarize: