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

Format bare gives no error when trying to import another parser #490

Closed
XenoS-ITA opened this issue Feb 25, 2024 · 4 comments · Fixed by #492
Closed

Format bare gives no error when trying to import another parser #490

XenoS-ITA opened this issue Feb 25, 2024 · 4 comments · Fixed by #492
Assignees

Comments

@XenoS-ITA
Copy link
Contributor

When trying to compile a file with peggy if you set the format to bare the import is simply ignored with no warning indicating that bare cannot import

Step to reproduce:

  • Create a paggy file that imports another file
  • Compile the following file using bare (npx peggy path/to/file.peggy --format bare)
  • The compiled file will not have imports but paggy will not warn you

What should come out:
An error or warning that the import was ignored.
Maybe for the api change the default format to commonjs or throw an error

@hildjj hildjj self-assigned this Feb 25, 2024
@hildjj
Copy link
Contributor

hildjj commented Feb 25, 2024

I don't think we can change the default format in the API; that would break existing code.

@XenoS-ITA
Copy link
Contributor Author

you are totally right, it could be done with the next big version also because the fact that the CLI has commonjs as default and the API as bare does not makes not too much logical sense (in my opinion)

@hildjj
Copy link
Contributor

hildjj commented Feb 25, 2024

Let's fix this, and we can then open another issue to revisit the default at the next breaking change.

@hildjj
Copy link
Contributor

hildjj commented Feb 25, 2024

I'm not going to cut a release just for this, since there is a work-around.

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 a pull request may close this issue.

2 participants