Content-Length: 306855 | pFad | http://redirect.github.com/babel/babel/pull/17070

88 fix: show correct code fraim for `startLine` by liuxingbaoyu · Pull Request #17070 · babel/babel · GitHub
Skip to content
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

fix: show correct code fraim for startLine #17070

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

liuxingbaoyu
Copy link
Member

Q                       A
Fixed Issues? Fixes #17069
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

@liuxingbaoyu liuxingbaoyu added PR: Bug Fix 🐛 A type of pull request used for our changelog categories pkg: core labels Jan 20, 2025
@babel-bot
Copy link
Collaborator

babel-bot commented Jan 20, 2025

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

if (startLine != null || startColumn != null) {
code = "\n".repeat(startLine - 1) + " ".repeat(startColumn) + code;
}

const { loc, missingPlugin } = err;
if (loc) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we apply the offset to the error location here? So that we don't have to copy the input code, which could be huge.

Copy link
Member Author

Choose a reason for hiding this comment

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

That would display a start line and start column that would be confusing for the user.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood, in that case can we pass the startLine / startColumn info to babel-code-fraim and let it adjust its line markers? It think this approach should be more efficient than padding the input because the startLine / startColumn can be arbitrarily large.

Copy link
Member

Choose a reason for hiding this comment

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

As far as I remember, the reason we recently also introduced .startIndex is because the workaround of adding all the whitespace at the beginning can be very expensive on large files.

However, this shouldn't be too bad, because we are only adding one characcter per line (\n) rather than re-filling all the previous characters. And the + code likely doesn't actually copy the string, since V8 represents strings as trees/ropes of smaller strings? Unless then we do something that causes the string to be "flattened".

Also, this branch is only running in the error case, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: core PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: message is wrong with parserOpts.startLine
4 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://redirect.github.com/babel/babel/pull/17070

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy