Skip to content

Remove multiline comments with compress option#16

Merged
pschroen merged 1 commit intoglslify:masterfrom
zakjan:patch-1
Oct 3, 2021
Merged

Remove multiline comments with compress option#16
pschroen merged 1 commit intoglslify:masterfrom
zakjan:patch-1

Conversation

@zakjan
Copy link
Copy Markdown
Contributor

@zakjan zakjan commented Oct 3, 2021

Currently multiline comments are not removed because . doesn't match \n.

s regex modifier is required for . to match \n.

@zakjan
Copy link
Copy Markdown
Contributor Author

zakjan commented Oct 3, 2021

Upstream PR vwochnik/rollup-plugin-glsl#7

@pschroen
Copy link
Copy Markdown
Member

pschroen commented Oct 3, 2021

Hi, the compress option should already remove multiline comments without the s regex modifier, as it already handles \r and \n in the expression.

For example, the test case uses glsl-noise/simplex/3d by checking if // Description from the 2nd line is removed or not.

Would you be able to provide the multiline comment that isn't being removed we could test against?

@zakjan
Copy link
Copy Markdown
Contributor Author

zakjan commented Oct 3, 2021

By multiline comment I mean /* ...\n... */, such comment is not removed.

@pschroen
Copy link
Copy Markdown
Member

pschroen commented Oct 3, 2021

Ah, I see, let's add a test case for that. Is there a glslify module you're using that isn't compressing, or your own comment?

@zakjan
Copy link
Copy Markdown
Contributor Author

zakjan commented Oct 3, 2021

It's custom GLSL code.

@pschroen
Copy link
Copy Markdown
Member

pschroen commented Oct 3, 2021

Alright, also getting a linting error:

13:26 error Parsing error: Invalid regular expression flag

Investigating...

@pschroen pschroen self-assigned this Oct 3, 2021
@pschroen
Copy link
Copy Markdown
Member

pschroen commented Oct 3, 2021

It appears as though the s regex modifier is new to ES2018, it works, just need to update the ESLint config.

I'll merge the PR and update the ESLint config.

@pschroen pschroen merged commit 8977ba7 into glslify:master Oct 3, 2021
@pschroen
Copy link
Copy Markdown
Member

pschroen commented Oct 3, 2021

Published rollup-plugin-glslify@1.2.1. If you can confirm your multiline comments are being removed now. Thanks!

@zakjan
Copy link
Copy Markdown
Contributor Author

zakjan commented Oct 3, 2021

Yes, it works, thanks!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants