fix comment parsing at start of file #28489
Conversation
* skip shebang if present (fixes: microsoft#28477) * don't parse trailing comments as they are also treated like leading comments
src/compiler/scanner.ts
Outdated
| let accumulator = initial; | ||
| if (pos === 0) { | ||
| if (collecting) { |
There was a problem hiding this comment.
I don't understand this check.. Whats the reason for this change?
My expectation was that we only skip Shebang if pos = 0 and follow rest of the algorithm as is. Is that not sufficient ?
There was a problem hiding this comment.
While writing the tests I noticed that comments in the first line of a file were returned as leading and trailing comment. So I made this small change to never return trailing comments at position 0
There was a problem hiding this comment.
@sheetalkamat would you prefer these changes to be split into two PRs (one to skip the shebang and one to not parse trailing comments at the start of the file)?
There was a problem hiding this comment.
I believe the reason this hasn't been necessary is that forEachLeadingCommentToEmit and forEachTrailingCommentToEmit in emitter.ts handle not emitting overlapping comment ranges. I don't think this change is necessary for now and we should stick to just skipping the shebang in this PR.
There was a problem hiding this comment.
Ok, so I'll revert this change and open an issue to discuss changing it in a follow-up PR.
src/compiler/scanner.ts
Outdated
| let accumulator = initial; | ||
| if (pos === 0) { | ||
| if (collecting) { |
There was a problem hiding this comment.
I believe the reason this hasn't been necessary is that forEachLeadingCommentToEmit and forEachTrailingCommentToEmit in emitter.ts handle not emitting overlapping comment ranges. I don't think this change is necessary for now and we should stick to just skipping the shebang in this PR.
|
@RyanCavanaugh do we want to take this for 3.3? If not I'll merge this in the afternoon. |
|
Thanks for the contribution! |
don't parse trailing comments to avoid duplicates that are also returned as leading comments