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

Improve sourcemaps resolve command output #1880

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

szokeasaurusrex
Copy link
Member

@szokeasaurusrex szokeasaurusrex commented Dec 29, 2023

This PR makes two improvements to the sourcemaps resolve command output:

  • The PR changes the output line and column values from zero-indexed to one-indexed values. By switching to one-indexed values, the output line and column numbers use the same scale as the input values and follow the standard convention for line and column numbers in source code files. This change fixes the off-by-one error reported in Source mapping is offset when using switch statements #1863.
  • The output text has been restructured and clarified to indicate that the sourcemap resolver locates the closest token to the line and column input that the user provides. With this change, it should be clear to users that the source and minified file line/column locations reported in the output are the locations of this nearest token that the sourcemap resolver locates. This change is specifically intended to help avoid confusion in cases where the nearest token may in a different location (perhaps even, on a different line) than the search location provided in the user's input.

Example

To demonstrate this change, we include an example below to demonstrate how the command output has changed. We use the sourcemap from the "Steps to Reproduce" in #1863 (comment).

Example command

$ sentry-cli sourcemaps resolve /path/to/example/sourcemap.js.map -l 6

Old output

source map path: "[...]"
source map type: regular
lookup line: 5, column: 0:
  name: not found
  source file: "/test.ts"
  source line: 5
  source column: 13
  minified line: 4
  minified column: 15
  source code:
    console.log("test2");
    
    switch (process.env.TEST) {
      case "test":
        throw new Error("test");
      default:
        throw new Error("default");

New output

source map path: "[...]"
source map type: regular

Searching for token nearest to line 6, column 1 in the minified file:

  Found the nearest token (unnamed) at line 5, column 16 in the minified file.

        - The same token is located at line 6, column 14 in source file /test.ts.


  Source code:
    console.log("test2");
    
    switch (process.env.TEST) {
      case "test":
        throw new Error("test");
      default:
        throw new Error("default");

Fixes GH-1863

Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

🎉 1-based offsets is so much better

If you want to be even fancier, you could use so colors/format to bold the numbers or something, but I believe things are already much better.

@szokeasaurusrex szokeasaurusrex merged commit c4f6b56 into master Jan 5, 2024
12 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/fix-sourcemaps-resolve branch January 5, 2024 09:29
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.

Source mapping is offset when using switch statements
2 participants