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

Allow vertical pipelines #10

Open
showell opened this issue Nov 7, 2019 · 6 comments
Open

Allow vertical pipelines #10

showell opened this issue Nov 7, 2019 · 6 comments
Milestone

Comments

@showell
Copy link

showell commented Nov 7, 2019

I would be nice to format code like this:

normalize lst =
    lst
        |> List.indexedMap Tuple.pair
        |> List.sortBy Tuple.second
        |> List.map Tuple.first
        |> List.indexedMap Tuple.pair
        |> List.sortBy Tuple.second
        |> List.map Tuple.first
        |> List.map (\n -> n + 1)

If there's already a way to generate code in that format, feel free to close, obviously.

@rupertlssmith
Copy link
Collaborator

rupertlssmith commented Nov 7, 2019

Been wondering what the best way of handling this is.

  1. Could add an option to the pretty printer { alwaysBreakPipeline = True }.
  2. Could just always break them regardless and not try to fit on single lines ever.
  3. Could allow a single |> but not 2 or more:
normalize lst =
    lst |> List.indexedMap Tuple.pair

normalize lst =
    lst
        |> List.indexedMap Tuple.pair
        |> List.sortBy Tuple.second
  1. Could add some rule based on a 'ribbon width'. If a pipeline takes more space than X characters then split it, where X is independant of the page width, so can be set to break pipelines more eagerly than the full page width.
-- This would still be allowed on 1 line, since it is short.
a |> c |> d |> d

@showell
Copy link
Author

showell commented Nov 7, 2019

I personally almost always go vertical, but some folks get really annoyed by line count, so I suspect option #1 or #2 would be more popular.

Option #5 is my favorite, which is something like verticalPipeline (but shorter name), which you could specify at the AST level, not the pretty printer. (And if you wanted total control, you could then override it in pretty.)

@rupertlssmith
Copy link
Collaborator

  1. Perhaps there could be an always-break function? Called break?
{-| Force the outermost level of an expression to be always broken accross lines. -}
break : Expression -> Expression

Implementation details... a little bit complex since its not just the outermost level, want to apply it all the way down a pipeline too.

@showell
Copy link
Author

showell commented Nov 7, 2019

You could do it on a per-line basis too. (It's somewhat painful for hand-written code, but folks could add their own helpers to the extent that they're even building this kind of code manually.)

162                 CG.NL <| lst
163                 [ CG.NL <| CG.apply [ indexedMap, pair ]
164                 , CG.NL <| CG.apply [ sortBy, second ]
165                 , CG.NL <| CG.apply [ map, first ]
166                 , CG.NL <| CG.apply [ indexedMap, pair ]
167                 , CG.NL <| CG.apply [ sortBy, second ]
168                 , CG.NL <| CG.apply [ map, first ]
169                 , CG.NL <| CG.apply [ map, incrLambda ]
170                 ]

@rupertlssmith rupertlssmith added this to the v5 milestone Dec 9, 2019
@rupertlssmith
Copy link
Collaborator

rupertlssmith commented Jun 8, 2020

The reason this is tricky to implement at the code generator API level, is that this builds on top of stil4m/elm-syntax, which does not leave room in its data model for a 'break' flag.

So to have an option in the code generator API for the user to choose if pipes are broken or not, means I need to store something in the DSL to instruct the pretty printer on how to print it. To implement this, I need to make a copy of elm-syntax and modify its data model to support this feature.

I could do this quite easily by just copying its codebase but not exposing it to get to the starting point. Would then work from there to figure out what parts of it should be exposed.

Would probably change the Node type from stil4m/elm-syntax to be Node a to allow arbitrary extra layout information to be added throughout the AST. It may even be possible to expose this in a way that users of the DSL can insert arbitrary extra information, if something nice can be done with that.

@rupertlssmith
Copy link
Collaborator

rupertlssmith commented Jun 8, 2020

@Akopella On slack expressed a preference for option 1.

Its a good idea to implement this flag, even if other solutuions are available, as it could override them. Some coding standards like to strictly enfore vertical pipes.

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

2 participants