-
Notifications
You must be signed in to change notification settings - Fork 12
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
Adds List.push and prepend #70
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.
Hm, fair point @lue-bird. Although I don't really like functions that are only pipeline optimized. Perhaps we could do something with the |
I don't think they are "only pipeline optimized". - |
@lue-bird ok, I've implemented the above. Are we happy for this to go in? |
These come up as missing in pipelines quite often. I see a lot of code like
which could be a lot nicer with