Skip to content

bug with parsing multiple comments #173

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

Closed
snake opened this issue Nov 4, 2019 · 2 comments · Fixed by #672 or #1169 · May be fixed by #663
Closed

bug with parsing multiple comments #173

snake opened this issue Nov 4, 2019 · 2 comments · Fixed by #672 or #1169 · May be fixed by #663

Comments

@snake
Copy link

snake commented Nov 4, 2019

When parsing a rule or csslist, trailing comments (those comments after the rule/list) are consumed as part of a final consumeWhitespace call, meaning we don't see these reported for the next rule in the set (or the next set).

Consider the following cases:

div {/*Find Me!*/left:10px; /*Find Me Too!*/text-align:left;}
Both of these comments should be returned (one for each rule), but the latter is not.

/*Find Me!*/div {left:10px; text-align:left;} /*Find Me Too!*/a {left:10px;}
Both of these comments should be returned (one for each csslist), but the latter is not.

I'd like this fixed so I can use this to parse RTL directives for css rules/lists reliably again. Moodle recently upgraded to 8.3.0 and this broke our RTL.

Please see the linked fix proposal.

snake added a commit to snake/PHP-CSS-Parser that referenced this issue Nov 4, 2019
This fixes MyIntervals#173.

Because of an eager consumption of whitespace, the rule and csslist
parsing would swallow a trailing comment, meaning the comment for the
next rule/list would be affected. This patch addresses this by not
consuming whitespace after a rule/list.
ziegenberg added a commit to ziegenberg/PHP-CSS-Parser that referenced this issue Aug 24, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes MyIntervals#173

Signed-off-by: Daniel Ziegenberg <[email protected]>
ziegenberg added a commit to ziegenberg/PHP-CSS-Parser that referenced this issue Aug 24, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes MyIntervals#173

Signed-off-by: Daniel Ziegenberg <[email protected]>
ziegenberg added a commit to ziegenberg/PHP-CSS-Parser that referenced this issue Aug 24, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes MyIntervals#173

Signed-off-by: Daniel Ziegenberg <[email protected]>
ziegenberg added a commit to ziegenberg/PHP-CSS-Parser that referenced this issue Aug 25, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes MyIntervals#173

Signed-off-by: Daniel Ziegenberg <[email protected]>
ziegenberg added a commit to ziegenberg/PHP-CSS-Parser that referenced this issue Aug 25, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes MyIntervals#173

Signed-off-by: Daniel Ziegenberg <[email protected]>
ziegenberg added a commit to ziegenberg/PHP-CSS-Parser that referenced this issue Aug 25, 2024
…s#671)

Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes MyIntervals#173

Signed-off-by: Daniel Ziegenberg <[email protected]>
oliverklee pushed a commit that referenced this issue Aug 25, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes #173

Signed-off-by: Daniel Ziegenberg <[email protected]>
oliverklee pushed a commit that referenced this issue Aug 25, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes #173

Signed-off-by: Daniel Ziegenberg <[email protected]>
oliverklee added a commit that referenced this issue Aug 25, 2024
Because of an eager consumption of whitespace, the rule parsing would
swallow a trailing comment, meaning the comment for the next rule would
be affected. This patch addresses this by only consuming real whitespace
without comments after a rule.

Fixes #173

Signed-off-by: Daniel Ziegenberg <[email protected]>
Co-authored-by: Daniel Ziegenberg <[email protected]>
@andrewnicols
Copy link

This should be reopened as #672 was reverted.

@oliverklee
Copy link
Contributor

Indeed, thanks! 🙏 Reopening.

@oliverklee oliverklee reopened this Mar 13, 2025
@oliverklee oliverklee added the bug label Mar 13, 2025
JakeQZ added a commit that referenced this issue Mar 15, 2025
- `Rule::parse()` will no longer consume anything after the semicolon
  terminating the rule - it does not belong to that rule;
- The whitespace and comments before a rule will be processed by
  `RuleSet::parseRuleSet()` and passed as a parameter to `Rule::parse()` -
  - This is only required while 'strict mode' parsing is an option,
    to avoid having an exception thrown during normal operation
    (i.e. when `Rule::parse()` encounters normal `}`
    as opposed to some other junk, which is not distinguished).

Fixes #173.
See also #672, #741.
JakeQZ added a commit that referenced this issue Mar 15, 2025
- `Rule::parse()` will no longer consume anything after the semicolon
  terminating the rule - it does not belong to that rule;
- The whitespace and comments before a rule will be processed by
  `RuleSet::parseRuleSet()` and passed as a parameter to `Rule::parse()` -
  - This is only required while 'strict mode' parsing is an option,
    to avoid having an exception thrown during normal operation
    (i.e. when `Rule::parse()` encounters normal `}`
    as opposed to some other junk, which is not distinguished).

Fixes #173.
See also #672, #741.
JakeQZ added a commit that referenced this issue Mar 16, 2025
This is the v8.x backport of #1169.

- `Rule::parse()` will no longer consume anything after the semicolon
  terminating the rule - it does not belong to that rule;
- The whitespace and comments before a rule will be processed by
  `RuleSet::parseRuleSet()` and passed as a parameter to `Rule::parse()` -
  - This is only required while 'strict mode' parsing is an option,
    to avoid having an exception thrown during normal operation
    (i.e. when `Rule::parse()` encounters normal `}`
    as opposed to some other junk, which is not distinguished).

Fixes #173.
See also #663, #672, #741.
JakeQZ added a commit that referenced this issue Mar 16, 2025
This is the v8.x backport of #1169.

- `Rule::parse()` will no longer consume anything after the semicolon
  terminating the rule - it does not belong to that rule;
- The whitespace and comments before a rule will be processed by
  `RuleSet::parseRuleSet()` and passed as a parameter to `Rule::parse()` -
  - This is only required while 'strict mode' parsing is an option,
    to avoid having an exception thrown during normal operation
    (i.e. when `Rule::parse()` encounters normal `}`
    as opposed to some other junk, which is not distinguished).

Fixes #173.
See also #663, #672, #741.
oliverklee pushed a commit that referenced this issue Mar 17, 2025
This is the v8.x backport of #1169.

- `Rule::parse()` will no longer consume anything after the semicolon
  terminating the rule - it does not belong to that rule;
- The whitespace and comments before a rule will be processed by
  `RuleSet::parseRuleSet()` and passed as a parameter to `Rule::parse()` -
  - This is only required while 'strict mode' parsing is an option,
    to avoid having an exception thrown during normal operation
    (i.e. when `Rule::parse()` encounters normal `}`
    as opposed to some other junk, which is not distinguished).

Fixes #173.
See also #663, #672, #741.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment