[BUGFIX] Fix comment parsing to support multiple comments
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
It would be nice to have this in an 8.6.1 release.
Battle plan as discussed in our call right now:
- 8.x We add a
consumeWhiteSpaceWithComments - 8.x
consumeWhiteSpaceredirects toconsumeWhiteSpaceWithComments, redirect it to the new method and mark it as deprecated - 8.x change the respective call to not call the
consume*method and have the corresponding lines directly instead - 9.0 remove/change the deprecated method
Battle plan as discussed in our call right now:
1. 8.x We add a `consumeWhiteSpaceWithComments` 2. 8.x `consumeWhiteSpace` redirects to `consumeWhiteSpaceWithComments`, redirect it to the new method and mark it as deprecated 3. 8.x change the respective call to not call the `consume*` method and have the corresponding lines directly instead 4. 9.0 remove/change the deprecated method
I don't think we should ever reintroduce a method with the same name as one that previously existed, with different default funcionality. It will catch out anyone doing a major update through several major versions, which happens even (and probably moreso) in sizeable companies as they defer the work required to support the update, deeming it low priority when updating requires some other changes.
So I think we may as well come up with a new method name now; maybe consumeWhitespaceOnly. (Unfortunately, PHP method names are not case sensitive, so we can't use consumeWhitespace.)
@ziegenberg Do we still need this PR now that #671/#672 have been merged?
I propose we move the discussion of the next steps here: #679
#173 has finally been fixed by #1169; however, the suggestions here seem good for consideration moving forwards.