Skip to content

Improve comment generation after compilation #15255

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.

Sign up for GitHub

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

Jump to bottom
Open
liuxingbaoyu wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Improve comment generation after compilation #15255

liuxingbaoyu wants to merge 5 commits into from

Conversation

@liuxingbaoyu
Copy link
Member

@liuxingbaoyu liuxingbaoyu commented Dec 7, 2022 •
edited
Loading

Q                       A
Fixed Issues? Fixes #15156, Fixes #11971, Closes #12558, Fixes #8507
Patch: Bug Fix? ?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? √
Documentation PR Link
Any Dependency Changes?
License MIT

@liuxingbaoyu liuxingbaoyu added PR: Polish 💅 A type of pull request used for our changelog categories pkg: generator area: comments pkg: core labels Dec 7, 2022
liuxingbaoyu
liuxingbaoyu commented Dec 7, 2022
View reviewed changes
packages/babel-generator/src/printer.ts
Comment on lines 950 to +951
const printNewLines =
isBlockComment &&
skipNewLines !== COMMENT_SKIP_NEWLINE.ALL &&
!this._noLineTerminator;
skipNewLines !== COMMENT_SKIP_NEWLINE.ALL && !this._noLineTerminator;
Copy link
Member Author

@liuxingbaoyu liuxingbaoyu Dec 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Most of the changes of tests come from here, from testing changes it looks ok,

@babel-bot
Copy link
Collaborator

babel-bot commented Dec 7, 2022 •
edited
Loading

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/53624/

@liuxingbaoyu
Copy link
Member Author

liuxingbaoyu commented Dec 7, 2022

I don't think this is a good approach, but it shouldn't be too bad.🤷

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories and removed PR: Polish 💅 A type of pull request used for our changelog categories labels Dec 7, 2022
@liuxingbaoyu
Copy link
Member Author

liuxingbaoyu commented Dec 7, 2022

I believe this has new unintended behavior such as

// This import xxx
import xxx

But that's nothing compared to improvement.
Before the first line comments were extremely easy to get lost or printed later, now they have been improved, as can be seen from the test changes.
https://babeljs.io/repl#?browsers=&build=&builtIns=false&corejs=3.21&spec=false&loose=false&code_lz=PTAEBcFMGdwKAJYFsAOB7ATuUAlSBDAY2wDMM0lQByDA4qgbiA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=false&presets=env&prettier=false&targets=&version=7.20.6&externalPlugins=&assumptions=%7B%7D

liuxingbaoyu
liuxingbaoyu commented Dec 7, 2022
View reviewed changes
packages/babel-core/test/fixtures/transformation/source-maps/input-source-map-complex/output.js
Comment on lines 85 to 88
//
//

//
Copy link
Member Author

@liuxingbaoyu liuxingbaoyu Dec 7, 2022

Choose a reason for hiding this comment

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

This could indicate a generator bug, since the input also contains this blank line, which was not printed correctly before, but that's not important.

@liuxingbaoyu liuxingbaoyu force-pushed the improve-transed-comments branch from fe3e5c0 to f19ba6e Compare December 8, 2022 01:38
liuxingbaoyu
liuxingbaoyu commented Dec 8, 2022
View reviewed changes
packages/babel-core/src/transformation/file/comments.ts
Comment on lines +59 to +65
if (
lastNode.loc &&
node.loc &&
commentLoc &&
commentLoc.start.line - lastNode.loc.end.line >=
node.loc.start.line - commentLoc.end.line
) {
Copy link
Member Author

@liuxingbaoyu liuxingbaoyu Dec 8, 2022

Choose a reason for hiding this comment

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

Such a situation should not exist for @babel/parser, but I don't know what the parser plugin is, anyway, it doesn't matter if we add judgment.

@liuxingbaoyu liuxingbaoyu mentioned this pull request Feb 16, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area: comments pkg: core pkg: generator PR: Bug Fix 🐛 A type of pull request used for our changelog categories

Projects

None yet

Milestone

No milestone

2 participants

@liuxingbaoyu @babel-bot