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

Allow no-whitespace and single-line comments (#66) #67

Merged
merged 1 commit into from Sep 12, 2020

Conversation

owst
Copy link
Contributor

@owst owst commented Sep 11, 2020

No description provided.

@@ -120,7 +120,7 @@

literal_delimiters = ']' | '}';

ascii_print = ((0x20..0x7e) - meta_char);
ascii_print = ((0x20..0x7e) - meta_char - '#');
Copy link
Collaborator

Choose a reason for hiding this comment

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

it took me a bit to figure out why this is necessary.

it is because the (ascii_print -- space)+ [...] literal matcher used to match up to and including pound signs, and the comment matcher only had a chance to fire if the literal matcher was interrupted by a space.

having the declarations of what can "safely" be considered a literal spread out is a little confusing, but that was already the case before this PR. i might look into consolidating this a bit later.

@jaynetics
Copy link
Collaborator

the build succeeded

@jaynetics jaynetics merged commit 58a00d6 into ammar:master Sep 12, 2020
jaynetics added a commit that referenced this pull request Sep 28, 2020
Issue #70.

The comment scanner was to greedy. In a way this bug always existed.

Comment-like patterns with a specific shape have always been scanned incorrectly in normal mode, e.g.

```ruby
/foo # \d
/
```

This was just very rare. Prior to the fix of issue #66 via PR #67, the comment scanner only fired for a limited, incomplete subset of valid comments like the one above. With the broadening of the scanner, this bug became much easier to hit upon.
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.

None yet

2 participants