Inline comments include line break

[mgrip]

Seems like when inline comments are parsed they include the line break - both in the comment.value and the comment.loc.end

[ichiriac]

Not sure that's a bug as the original PHP token used for inline comments also includes the line break. The fix would be counter intuitive, to be clean it should be changed from the tokeniser.

How does it breaks something on prettier ?

[mgrip]

We got around it for now by manually stripping out the line break and updating the end location (here). For reference this is the same thing in the JS parser: https://astexplorer.net/ - you can see it doesn't include the new line. Basically it throws off how core prettier assigns comments. It thinks its a leading comment for the newline instead of a trailing comment for the actual line it was written on

[mgrip]

I'm fine keeping as is for now since our workaround seems pretty safe and not all that complicated. Just wanted to flag in case its something you think should actually be changed in the parser

[ichiriac]

Ok, got it. In fact I try to stick to original PHP behavior, and sometimes they make strange decisions.

BTW, don't forget to strip mac & windows returns, just add another check after the first if over "\r" - so you will handle "\n", "\r\n", "\r"

I'll try to look if I can do something directly into the AST even if it would be inconsistent with tokens.

[mgrip]

Ooh good call, I'll update that. And sounds good, thanks for looking into it!

[ichiriac]

Hi @mgrip - does it fixed ? If not, it may introduce breaking changes ? If yes, it should be released on 3.0.0

[mgrip]

Thanks for following up @ichiriac! I've since stepped back from the prettier php project, I'll defer to @evilebottnawi or @czosel!

[czosel]

I tried following up on this a little, but can't report anything conclusive. As far as I can tell, we're ok with the way things are currently handled, so I wouldn't prioritize this for 3.0.0.

[ichiriac]

ok @czosel, thx for the feedback