search

tedious / JShrink

Javascript Minifier built in PHP
http://www.tedivm.com
BSD 3-Clause "New" or "Revised" License
751 stars 152 forks source link

To identify correct end of the regex #111

Closed CGLOB closed 1 year ago

CGLOB commented 1 year ago

When a regular expression is detected this saveRegex crawls for the correct end of it and will saves the whole regex.

tedivm commented 1 year ago

Thanks for the PR! This weekend I'll get tests working again so we can confirm no regressions.

tedivm commented 1 year ago

I'm rather confused as to why tests aren't running- can you try closing and reopening the pull request?

CGLOB commented 1 year ago

sure

tedivm commented 1 year ago

Okay lets try something else- can you make a new branch in your repository and open the pull request from that branch? I think the issue is that it's not linking the tests in your repository (where they're technically running now) to the pull request in part because they're running in your default branch for the repo.

CGLOB commented 1 year ago

ok on it

tedivm commented 1 year ago

Once that is done I would also like it is you could add a test case for this bug to the test suite to make sure that this actually resolves it and so that future changes don't cause a regression.

To do that put a file named "regex_end_string.js" to-

./JShrink/tree/master/tests/Resources/jshrink/input/

And then add a second file with the expected output to-

./JShrink/tree/master/tests/Resources/jshrink/output/

Ideally this example should be one that failed before your PR, and that is also resolved by that PR. If you have any trouble please let me know!

CGLOB commented 1 year ago

Once that is done I would also like it is you could add a test case for this bug to the test suite to make sure that this actually resolves it and so that future changes don't cause a regression.

To do that put a file named "regex_end_string.js" to-

./JShrink/tree/master/tests/Resources/jshrink/input/

And then add a second file with the expected output to-

./JShrink/tree/master/tests/Resources/jshrink/output/

Ideally this example should be one that failed before your PR, and that is also resolved by that PR. If you have any trouble please let me know!

Sure i will add that and will let you know if any issue. Thanks.

CGLOB commented 1 year ago

Okay lets try something else- can you make a new branch in your repository and open the pull request from that branch? I think the issue is that it's not linking the tests in your repository (where they're technically running now) to the pull request in part because they're running in your default branch for the repo.

First i will open new PR then i will work on the test.

CGLOB commented 1 year ago

Please check this PR https://github.com/tedious/JShrink/pull/116

tedivm commented 1 year ago

Okay I got tests running here!

CGLOB commented 1 year ago

with your latest changes the issue no longer reproducible. So my solution no longer needed. Hence i am closing this PR. Thanks for your attention and help.

tedivm commented 1 year ago

It would be great if you could share an example of what broke, either here or in a pull request, so that we can protect against regressions.