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

Error nodes, ParseErrors get source-mapped location #1262

Merged
merged 8 commits into from
May 23, 2024
Merged

Conversation

edemaine
Copy link
Collaborator

@edemaine edemaine commented May 21, 2024

Fixes #1251 (modulo TODOs below)

The basic idea is, during generate, to translate an "Error" node into a ParseError object with the current source-mapped location. This only works with sourceMap enabled, so in some cases I call generate a second time in case of error + no source maps. I also had to tweak the source mapping code to track srcLine and srcColumn too. (Early versions of this PR confused the source and output line/column numbers, but this is now resolved.)

Now options.errors becomes a list of ParseError objects instead of "Error" nodes. So far I've just consumed these in main.civet when building ParseErrors, which now has the array of ParseError objects, and gives line/column feedback in the CLI or testCase.

TODO (possibly in future PRs):

  • Playground line/column highlighting should use these line/column numbers
  • VSCode plugin should use these line/column numbers
  • Discord bot should use these line/column numbers

Prerequisite:

Relies on DanielXMoore/Hera#18 so tests won't work without npm link until that gets released.

Examples:

Here's an example of it working in Playground:

image

Here's an example of it working in VSCode:

image

Here's an example of it working with Discord bot:

image

Here's an example of it working in the CLI (improved error display to match Playground and bot):

image

Other changes:

  • Migrate from source/util.civet to Hera's source/sourcemap.civet. This was left as a TODO. Notably, changed the export from util to sourcemap.
  • SourceMap now directly exported
  • ParseError now exported
  • isCompileError checks for ParseError and ParseErrors
  • API change: Calling Civet.compile with errors: [] already in options will prevent ParseErrors from being thrown. This is important for e.g. the LSP which wants a compiled version in addition to the errors. Previously, you could achieve this behavior only by calling compile with ast: true and then calling generate separately.
  • CLI error display now shows where the error occurs with ^ instead of just line and column numbers (and no more stack trace)

Copy link
Contributor

@STRd6 STRd6 left a comment

Choose a reason for hiding this comment

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

Thanks, this is a great cleanup of a many sourcemapping TODOs!

Maybe eventually Hera could import the sourcemap class from Civet so it's no longer duplicated.

@edemaine
Copy link
Collaborator Author

Or a separate @danielx/sourcemap package? But yeah, Hera already depends on Civet, so makes sense to just use in that way.

@edemaine edemaine merged commit 209582c into main May 23, 2024
2 checks passed
@edemaine edemaine deleted the error-map branch May 23, 2024 15:51
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.

Add source location to error nodes
2 participants