Skip to content

Add line number magic comment support #23549

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

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

Conversation

Linyxus
Copy link
Contributor

@Linyxus Linyxus commented Jul 16, 2025

  • Added compiler setting to configure magic line number comments
  • Implemented line number offset reporting when magic comments are present

fixes #21919.

@bishabosha
Copy link
Member

@lihaoyi did you say you need two markers now? one for the end as well?

@bishabosha
Copy link
Member

for the mill use case in the past at least it needed 1 marker to tell the reporter the "original file" (where reporter should say error occurs only when its after the marker) and then one for start of user code, but mill also does insert code after the user script (so i guess another marker would tell the reporter that code after that is in the augmented file)

@Linyxus
Copy link
Contributor Author

Linyxus commented Jul 17, 2025

I see. So the reporter should report file location according to that marker as well.

@Linyxus
Copy link
Contributor Author

Linyxus commented Jul 17, 2025

I am not quite sure whether this PR is the right direction. The special handling logic is in ErrorRendering.scala, which offsets the line number at the presence of a "magic comment" when rendering the source position. But the SourcePosition of the error still points to the augmented script with the old line numbers. Not sure whether this is sufficient for addressing the use cases. WDYT? @bishabosha

@lihaoyi
Copy link
Contributor

lihaoyi commented Jul 17, 2025

@bishabosha we only really care about the marker for the start of the file's user-code, since the goal is to appropriately offset the line numbers, and a marker to indicate the full path of the original source file the user-code originated from. Mill uses separate markers for each:

//SOURCECODE_ORIGINAL_FILE_PATH=/Users/lihaoyi/Github/mill/build.mill
//SOURCECODE_ORIGINAL_CODE_START_MARKER

But I suppose we could just have a single marker that serves both purposes.

We don't really need to have an marker for the end of the user-code, since that doesn't affect line numbers in user code

@Linyxus to satisfy the original ticket, we should change the source position not just for error messages, but also info, warning, as well as macros like sourcecode.File and sourcecode.Line. There should not be any positions pointing towards the generated wrapped code files. We probably need to adjust the source locations early on during parsing to ensure the adjustment takes effect for all user-facing reporting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way to offset line numbers in wrapped scripts
3 participants