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

Add support for parsing import statement start & end #41

Closed

Conversation

FredKSchott
Copy link
Contributor

Resolves one half of #36

  • Adds statement_start (ss) and statement_end (se) to the data object for standard imports.
  • For dynamic imports & import.meta, both values return -1.

@FredKSchott FredKSchott force-pushed the add-statement-start-end branch from 8e8206f to 3da7e16 Compare January 24, 2020 20:00
README.md Show resolved Hide resolved
src/lexer.c Outdated Show resolved Hide resolved
@FredKSchott
Copy link
Contributor Author

I was able to see and fix all compilation errors, but clang was having trouble actually building the thing. I think the make file is using flags that my version of clang doesn't support.

If you can check this out locally and compile + confirm that tests are passing, that would be great. Otherwise, when I next have time I can try to fiddle with updating clang (can you let me know what version you use / is needed?).

Copy link
Owner

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Great to see this, thanks.

@FredKSchott yes getting the build setup is the hard part as mentioned here. I'm just off on a holiday right now, so won't be able to look at this for the next week or so unfortunately. It would help a lot for review if you can try confirm the build and tests are passing here.

The WASI SDK from https://github.com/CraneStation/wasi-sdk/releases needs to be downloaded to be referenced from https://github.com/guybedford/es-module-lexer/blob/master/Makefile#L6. Getting the exact path level there is a gotcha - one nested folder too deep and it blows up, until it works. Any Clang 8 version should work, or you can use the WASI SDK Clang build which includes everything needed. See https://github.com/guybedford/es-module-lexer#building for more info too.

Makefile Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
src/lexer.c Outdated Show resolved Hide resolved
src/lexer.h Outdated Show resolved Hide resolved
@FredKSchott
Copy link
Contributor Author

FredKSchott commented Jan 26, 2020

@guybedford Okay, comments responded to, it's passing compilation but I cannot for the life of me get it building/linking successfully. I've tried on both my local mac (ld: unknown option: -z) and a clean Ubuntu machine on AWS (../wasi-sdk-6/opt/wasi-sdk/share/wasi-sysroot/include/wasi/core.h:15:2: error: <wasi/core.h> is only supported on WASI platforms.). I'm sure that I did something stupid installing the right version of clang/etc., but unfortunately I don't have any more time available to spend on this for the moment.

@guybedford
Copy link
Owner

@FredKSchott thanks for trying, yes I know it's a pain! All the more reason to migrate this project to Ziglang as I say, in due course :)

The build ran smoothly with just one or two bugs that needed correcting. I've gone ahead and landed on master, will push out a release shortly. Hope it helps you there! I'm sure there will be more bugs in this project, so further contributions back in future would be appreciated - this isn't a huge priority for me so can't guarantee ongoing support, but will do all I can to facilitate any PRs or interested maintainers.

@guybedford guybedford closed this Feb 1, 2020
@guybedford
Copy link
Owner

Released in 0.3.14.

@FredKSchott
Copy link
Contributor Author

Ah thanks for getting this over the line @guybedford. I'll start to integrate it into my project and let you know if anything comes up in the future. Happy to contribute some PRs for anything that I find if time allows.

Thanks for the great project/package!

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.

2 participants