Skip to content

Fix ExprHash internal error when MD5 is used#8609

Open
Dev-Xeiji wants to merge 3 commits intoSkriptLang:dev/patchfrom
Dev-Xeiji:patch-1
Open

Fix ExprHash internal error when MD5 is used#8609
Dev-Xeiji wants to merge 3 commits intoSkriptLang:dev/patchfrom
Dev-Xeiji:patch-1

Conversation

@Dev-Xeiji
Copy link
Copy Markdown

@Dev-Xeiji Dev-Xeiji commented May 1, 2026

Added exception handling for MD5 warning suppression.

Problem

#8474 (Error was caused by trying to give the warning, regards with @bluelhf)

Solution

Adopted the recommended fix by @APickledWalrus #8474 (comment)

Testing Completed

image

Supporting Information


Completes: #8474
Related: none
AI assistance: none

Added exception handling for MD5 warning suppression.
@Dev-Xeiji Dev-Xeiji requested a review from a team as a code owner May 1, 2026 12:07
@Dev-Xeiji Dev-Xeiji requested review from UnderscoreTud and erenkarakal and removed request for a team May 1, 2026 12:07
Copy link
Copy Markdown
Contributor

@bluelhf bluelhf left a comment

Choose a reason for hiding this comment

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

This should use the approach suggested by @APickledWalrus with a condition rather than try-catch, because we don't want to suppress other types of exceptions and because the code for that will look cleaner.

@Dev-Xeiji
Copy link
Copy Markdown
Author

This should use the approach suggested by @APickledWalrus with a condition rather than try-catch, because we don't want to suppress other types of exceptions and because the code for that will look cleaner.

Ohh alright, I was just actually looking to some related file when doing this, that resulted on this approach. Will go update it to what's suggested

@Dev-Xeiji Dev-Xeiji changed the base branch from master to dev/patch May 1, 2026 14:01
@Dev-Xeiji Dev-Xeiji requested a review from bluelhf May 1, 2026 14:27
Copy link
Copy Markdown
Contributor

@bluelhf bluelhf left a comment

Choose a reason for hiding this comment

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

lgtm :)

@skriptlang-automation skriptlang-automation Bot added the needs reviews A PR that needs additional reviews label May 5, 2026
Copy link
Copy Markdown
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.

I might save the result of getParser() rather than call it twice, but this looks good.

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

Labels

needs reviews A PR that needs additional reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants