-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
take
-> pop
#12678
take
-> pop
#12678
Conversation
#12600 and in https://forum.nim-lang.org/t/5499 indicates that everyone is happy/happier with ``pop``. This just renames the brand new ``take``s to ``pop`` and installs inline aliases/wrappers to preserve ``Table.take`` and ``TableRef.take``. Update apis.rst to try to maintain consistency of remove-and-return procs.
NOTE: for the various tables it might also be nice to have an "unkeyed pop" that like |
result = t[].take(key, val) | ||
result = t[].pop(key, val) | ||
|
||
proc take*[A, B](t: TableRef[A, B], key: A, val: var B): bool {.inline.} = |
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 would deprecate this while you're at it.
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 don't disagree as indicated by prior PR thread, but @Araq reacted negatively to that idea in the linked forum thread. So, I didn't.
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.
It may simply be the price we pay for not fixing Table.take
for 2 years with a 1.0 transition in there to maintain it forever as an alias. I did demote it in the docs, though. After some purgatory period in that alias-in-docs state, it could maybe become deprecated for another period before final removal. That might or might not be a good model for how to revise stdlib things in the 1.0 era.
This is confusing, as seq.pop returns the element while, take returns a bool. In my opinion there are different operations and need to have different names. |
Well, Nim has all kinds of type-overloaded APIs and users of APIs do need to be aware of the parameter types. Unlike |
The above said, there has been a lot of back & forth about |
|
@Araq - anything else you need/want? |
Discussion both in
#12600
and in
https://forum.nim-lang.org/t/5499
indicates that everyone is happy/happier with
pop
.This just renames the brand new
take
s topop
and installs inlinealiases/wrappers to preserve
Table.take
andTableRef.take
.Update apis.rst to try to maintain consistency of remove-and-return procs.