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

Heregex Comment #5435

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Heregex Comment #5435

wants to merge 5 commits into from

Conversation

STRd6
Copy link
Contributor

@STRd6 STRd6 commented Nov 26, 2022

Fixes #5428

This fix is simple enough but it causes a substantial change in behavior for Heregexes.

# inside of a character class shouldn't be considered a comment. By treating # without whitespace in front as non-comments we could have slight compatability with Python.

There are other places in the CoffeeScript source where people avoided escaping the # by keeping it next to non-whitespace characters even outside of a character class. This is different than how Python does it and is probably a bug but maybe it is too late with the de facto CoffeeScript2 behavior.

I'm not sure if this should be merged in since it changes the behavior quite a lot. Maybe another one for the CoffeeScript3/Civet bucket.

Refs

Python Docs: https://docs.python.org/3/library/re.html#re.X

Fixes jashkenas#5428

This fix is simple enough but it causes a substantial change in behavior for Heregexes.

`#` inside of a character class shouldn't be considered a comment. By treating `#`
without whitespace in front as non-comments we could have slight compatability with
Python.

There are other places in the CoffeeScript source where people avoided escaping the `#`
by keeping it next to non-whitespace characters even outside of a character class. This
is different than how Python does it and is probably a bug but maybe it is too late with
the de facto CoffeeScript2 behavior.

I'm not sure if this should be merged in since it changes the behavior quite a lot. Maybe
another one for the CoffeeScript3/Civet bucket.

Refs
---
Python Docs: https://docs.python.org/3/library/re.html#re.X
@STRd6
Copy link
Contributor Author

STRd6 commented Nov 26, 2022

GitHub auto-closed when I rebased so re-opening.

@STRd6 STRd6 reopened this Nov 26, 2022
@STRd6
Copy link
Contributor Author

STRd6 commented Nov 27, 2022

The interpolation test that was changed is interesting...

It's inserting a string containing a single backslash into the RegExp ahead of an escaped #. The current behavior is to keep the single backslash which is escaping the # in the output which then combines with another single backslash from the interpolation which causes the output to be an escaped backslash. There's no comment about the intent other than TODO improve heregex interpolation tests so I'm not sure if this existing behavior is desired.

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.

Bug: Heregex escaped space before comment includes comment in transpiled regex
1 participant