Skip to content

[RFC] Add clamp function #7191

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

Closed
wants to merge 28 commits into from
Closed

Conversation

thinkverse
Copy link

This PR adds an implementation for the RFC: https://wiki.php.net/rfc/clamp. any improvements are welcomed.

If you know how this function can be made faster and less cost-effective don't hesitate to point it out. 👍

@cmb69 cmb69 added the RFC label Jun 24, 2021
@thinkverse
Copy link
Author

Is there a cleaner way to handle NAN? Because I don't like the way I implemented that. 🤔  Also, is NAN a type in and of itself since technically it's a float? Not sure if the TypeError message is the correct wording here "clamp(): Argument #2 ($min) cannot be of type NAN".

Remove useless NAN check
Update clamp tests
@nikic nikic added this to the PHP 8.1 milestone Jul 6, 2021
@thinkverse
Copy link
Author

@nikic this RFC missed the voting window and can be removed from the 8.1 milestone, the proposed version is now targeting the 8.2 release.

@nikic nikic removed this from the PHP 8.1 milestone Jul 8, 2021
@ryangjchandler
Copy link

@thinkverse Nice work man!

@iluuu1994
Copy link
Member

@thinkverse Are you still interested in pursuing this for PHP 8.2?

@thinkverse
Copy link
Author

Are you still interested in pursuing this for PHP 8.2?

Yeah. Nikita helped with the NaN issue a while back so that should be considered solved. If there are no other glaring issues left it should be OK to open up voting on I think.

I've been holding off until other more major RFCs' have been voted on since this could be considered a low priority one compared to the rest.

@otengkwame
Copy link

The clamp() function could be used optionally in userland implementations. Looking forward to is addition in PHP 8.2 or in future

@iluuu1994
Copy link
Member

@thinkverse Any progress? Remember that feature freeze is very soon.

@danilopolani
Copy link

Great work! Looking forward to seeing it implemented in PHP 8.3 hopefully 🤞

@schodemeiss
Copy link

Is this function likely to make it into 8.3? Any reason this hasn't gone to a vote yet?

@sba
Copy link

sba commented Jun 12, 2023

Would be great if the clamp function makes it into php 8.3!

@thinkverse
Copy link
Author

I have made the difficult decision to withdraw the RFC, I currently don't have the resources or bandwidth for it. If anyone would like to take it over they are more than welcome to, I apologize to anyone who was looking forward to this. 🙏

@thinkverse thinkverse closed this Jun 12, 2023
@sba
Copy link

sba commented Jun 12, 2023

I have made the difficult decision to withdraw the RFC, I currently don't have the resources or bandwidth for it. If anyone would like to take it over they are more than welcome to, I apologize to anyone who was looking forward to this. 🙏

pity, that would be a nice feature. But I can well understand your decision. Unfortunately, I have neither resources nor experience with php-RFC, so I can't step in..

@vrubleg
Copy link

vrubleg commented Sep 29, 2024

I was actually surprised that it's still not in PHP. Sad that it didn't make it.

@kylekatarnls
Copy link
Contributor

I'm giving it another try (#19434) reusing your initial work @thinkverse, your comments on it are welcome 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.