Skip to content

Limit literal specification regex pattern to improve matching behavior. #8068

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

Open
wants to merge 5 commits into
base: dev/patch
Choose a base branch
from

Conversation

sovdeeth
Copy link
Member

Problem

Literal specification didn't work so well in function parameters: func(red, black (color)) would fail to parse.

Solution

Enforces stricter regex match rules (no {}()"' and so on in the literal, and only letters, numbers, and spaces in the codename).
Also ensures the two parseSingleExpr implementations are unified in behavior, as previously only one would attempt specified literals. Pulled duplicate code into a helper method.

Testing Completed

added to literal specification.sk

Supporting Information


Completes: none
Related: none

@sovdeeth sovdeeth requested review from a team as code owners July 22, 2025 00:36
@sovdeeth sovdeeth requested review from UnderscoreTud and removed request for a team July 22, 2025 00:36
@sovdeeth sovdeeth added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label Jul 22, 2025
@sovdeeth sovdeeth requested review from Burbulinis and removed request for a team July 22, 2025 00:36
@skriptlang-automation skriptlang-automation bot added the needs reviews A PR that needs additional reviews label Jul 22, 2025
@sovdeeth sovdeeth moved this to In Review in 2.12 Releases Jul 22, 2025
Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

Looks good

Comment on lines +717 to +719
if (allowUnparsedLiteral && containsObjectClass)
//noinspection unchecked
return (Expression<? extends T>) getUnparsedLiteral(log, error);
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, does this need to be after the string literal check? My concern is that something like "%object/string%" wouldn't work right (it would always return an unparsed literal).

Maybe the string check needs to go in loop above instead?

@APickledWalrus APickledWalrus linked an issue Jul 24, 2025 that may be closed by this pull request
1 task
@APickledWalrus APickledWalrus linked an issue Jul 24, 2025 that may be closed by this pull request
1 task
@sovdeeth sovdeeth changed the title Patch/litspecfuncparse Limit literal specification regex pattern to improve matching behavior. Jul 25, 2025
@skriptlang-automation skriptlang-automation bot added the patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version. label Jul 30, 2025
@github-project-automation github-project-automation bot moved this from In Review to Awaiting Merge in 2.12 Releases Jul 30, 2025
@skriptlang-automation skriptlang-automation bot removed the needs reviews A PR that needs additional reviews label Jul 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. patch-ready A PR/issue that has been approved and is ready to be merged/closed for the next patch version.
Projects
Status: Awaiting Merge
Development

Successfully merging this pull request may close these issues.

4 participants