Skip to content
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

fixed method toFile(), param "fileOut" jpeg2000 file extension check #3674

Closed
wants to merge 1 commit into from

Conversation

bianjunjie1981
Copy link
Contributor

fixed method toFile(), param "fileOut" jpeg2000 file extension check

The RE matches the file extension directly rather than the entire path,
The result will be much closer to correct.

@lovell
Copy link
Owner

lovell commented May 22, 2023

Thank you very much for the PR.

Would it be worth altering the regex to ensure the \. is treated independently of the | alternate capture, perhaps a bit like the following (untested)?

- const jp2Regex = /\.jp[2x]|j2[kc]$/i;
+ const jp2Regex = /\.(jp[2x]|j2[kc])$/i;

A suitable filename for testing purposes might be test.failj2c, and if you're able to add a unit test to help prevent future regression that would be great.

@bianjunjie1981
Copy link
Contributor Author

@lovell

To get closer to correct, modifying the regular expression itself is needed.
I don't have enough test cases to cover all possible scenarios.
You can only wait for the user to discover the error.

lovell pushed a commit that referenced this pull request Jun 5, 2023
lovell added a commit that referenced this pull request Jun 5, 2023
@lovell lovell added this to the v0.32.2 milestone Jun 5, 2023
@lovell
Copy link
Owner

lovell commented Jun 5, 2023

Landed as 7e6a70a - thank you very much for the PR - this will be in v0.32.2.

@lovell lovell closed this Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants