Skip to content

Conversation

@gampleman
Copy link
Collaborator

These come up as missing in pipelines quite often. I see a lot of code like

tags =
     node.tags
          |> Maybe.withDefault []
          |> (\tags -> tags ++ [ newTag.label ])
          |> Just

which could be a lot nicer with

tags =
     node.tags
          |> Maybe.withDefault []
          |> List.Extra.push newTag.label
          |> Just

Copy link
Collaborator

@lue-bird lue-bird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the prepend name confusing as hell in pipelines (what it's supposed to be used in)

[ 0 ] |> List.prepend [ 1 ] --> [ 0, 1 ]
-- "Wait I though it would prepend 1?

I would strongly suggest to instead introduce two functions prependTo and appendTo where the "to" signals it's use in a pipeline and what the "base list" is.

[ 0 ] |> prependTo [ 1 ] --> [ 0, 1 ]
[ 0 ] |> appendTo [ 1 ] --> [ 1, 0 ]

(used by minibill's elm-rope) but I'd also be fine with names like pushList-consList or appendSuffix-appendPrefix and similar.

@gampleman
Copy link
Collaborator Author

Hm, fair point @lue-bird. Although I don't really like functions that are only pipeline optimized. Perhaps we could do something with the l and r suffix? Like appendl and appendr?

@lue-bird
Copy link
Collaborator

lue-bird commented Feb 18, 2025

I don't think they are "only pipeline optimized".
E.g. remainderBy 3 x is still much more obvious than remainder x 3.
So I'd rather write prependTo [ 1 ] [ 0 ] than prepend [ 1 ] [ 0 ] (even though I can't really see many reasons to use prepend rather than append if you already call the function with both lists). Do you agree?

-l/-r IMO have the same problem as foldl/-r etc, it takes a bit of mental overhead to decipher and still is kind of ambiguous which is exactly the kind of issue a more rarely used helper shouldn't have

@gampleman
Copy link
Collaborator Author

@lue-bird ok, I've implemented the above. Are we happy for this to go in?

@gampleman gampleman merged commit aeb797c into master May 19, 2025
4 checks passed
@gampleman gampleman deleted the list-push branch May 19, 2025 08:43
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 this pull request may close these issues.

4 participants