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 no-pkg-flags-to-cmj test #391

Merged
merged 4 commits into from
Oct 28, 2022
Merged

Add no-pkg-flags-to-cmj test #391

merged 4 commits into from
Oct 28, 2022

Conversation

anmonteiro
Copy link
Member

Re-hash of #386

@anmonteiro anmonteiro marked this pull request as ready for review October 28, 2022 05:14

$ melc -bs-package-output lib/ lib/.objs/melange/a.cmj -o output/lib/a.js

If we pass some file extension or module system in `-bs-package-output`, it
Copy link
Member Author

Choose a reason for hiding this comment

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

@jchavarri I actually changed this. If you pass bs-package-output with package specs Melange will use that. The correct flag to use to control JS output at this stage is -bs-module-type

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good.

re: passing -o with file extension and -bs-package-output, I am not sure if we should fail? It's a bit confusing that user can pass file extension in two places.

Copy link
Member Author

Choose a reason for hiding this comment

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

it's indeed a bit confusing, but I think we don't need to care about that:

  • the CLI interface is for "expert" usage
  • people will mostly interact with melange through mel or dune

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that's one (maybe last?) thing to improve on later. I think the code is a lot cleaner after #384, as there's now a separation between the "package specs" (as recorded in the .cmj -- what's the relative path of this module?) and the output information we'll need for emission ("what module system and JS suffix should this file have?").

Copy link
Member

@jchavarri jchavarri left a comment

Choose a reason for hiding this comment

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

I only took a quick look, but the internal split between batch and separate compilation sounds like a great idea.

test/no-package-flags-to-cmj.t Outdated Show resolved Hide resolved
Co-authored-by: Javier Chávarri <javier.chavarri@gmail.com>
@anmonteiro anmonteiro merged commit b481d7b into main Oct 28, 2022
@anmonteiro anmonteiro deleted the anmonteiro/javi/386 branch October 28, 2022 09:05
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.

None yet

2 participants