Skip to content

Split ExprSpecialNumber into separate literals, make 'value' optional #7965

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

Merged

Conversation

sovdeeth
Copy link
Member

@sovdeeth sovdeeth commented Jun 20, 2025

Problem

ExprSpecialNumber made it a bit confusing to figure out that infinity/NaN values were present, wasn't a literal, and enforced the usage of value in every pattern, which was irritating. infinity should be just as valid as infinity value.

Solution

Replaces ExprSpecialNumber with three literals, LitInfinity, LitNegativeInfinity, and LitNaN. There didn't seem to be a nice reliable method to make a single literal have pattern-dependent values. It would be fine to have the value determined in init in most cases, but since every other literal doesn't require init to be called, it seemed unwise to rely on that.

The x value patterns have been replaced with x [value] and the negative and positive were added as prefixes for infinity.

Testing Completed

Existing tests suffice.

Supporting Information

In cases with no context, like set {a} to infinity, this changes behavior from using the infinity enchant to the value of infinity. Given that the enchant can also be referenced by infinity 1, I don't think this is a big issue. However, it's likely a lot more people use the enchant than the value, so we may want to explicitly maintain this behavior and allow positive infinity [value] and infinity value as the patterns.


Completes: none
Related: none

@sovdeeth sovdeeth requested a review from a team as a code owner June 20, 2025 20:44
@sovdeeth sovdeeth removed the request for review from a team June 20, 2025 20:44
@sovdeeth sovdeeth added the enhancement Feature request, an issue about something that could be improved, or a PR improving something. label Jun 20, 2025
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Jun 20, 2025
@sovdeeth sovdeeth moved this to In Review in 2.12 Releases Jun 20, 2025
@sovdeeth sovdeeth added the breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) label Jun 20, 2025
@skriptlang-automation skriptlang-automation bot added feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. and removed needs reviews A PR that needs additional reviews labels Jun 20, 2025
Copy link
Member

@erenkarakal erenkarakal left a comment

Choose a reason for hiding this comment

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

infinity value sounds fine but value of infinity sounds a bit weird. maybe im too sleepy

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.12 Releases Jun 21, 2025
@sovdeeth sovdeeth added the don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. label Jun 25, 2025
@sovdeeth sovdeeth removed breaking changes Pull or feature requests that contain breaking changes (API, syntax, etc.) don't merge me !! For pull requests that should not be merged due to some outstanding dispute, conflict or dependency. labels Jul 1, 2025
@sovdeeth
Copy link
Member Author

sovdeeth commented Jul 1, 2025

removed breaking changes by enforcing value if the only other keyword is infinity

@APickledWalrus APickledWalrus merged commit 7332d07 into SkriptLang:dev/feature Jul 1, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this from Awaiting Merge to Done in 2.12 Releases Jul 1, 2025
@skriptlang-automation skriptlang-automation bot added completed The issue has been fully resolved and the change will be in the next Skript update. and removed feature-ready A PR/issue that has been approved, tested and can be merged/closed in the next feature version. labels Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completed The issue has been fully resolved and the change will be in the next Skript update. enhancement Feature request, an issue about something that could be improved, or a PR improving something.
Projects
Status: Done - Released
Development

Successfully merging this pull request may close these issues.

5 participants