-
Notifications
You must be signed in to change notification settings - Fork 50
Allow any module name #182
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
Conversation
Seems to work. Fixes purescript#174
Ah, this behaves oddly if you give the module a name of an existing module in the package set. For example, if I call the current module |
This now seems to work even if I try to redefine an existing module. I haven't been able to think of a way that rewriting the module name like this would be an issue, it seems like a slightly nicer UX for what is essentially the same idea (only modules with a specific module name are allowed). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nice improvement. We’ll have to revisit this if Try PureScript ever supports multiple files / modules but that’s a faraway future.
-- Rewrite the module name to "$Main" in order to ensure that the | ||
-- module name doesn't clash with any existing module names in | ||
-- the package set. | ||
let rewriteModuleName (P.Module ss coms _ decls refs) = P.Module ss coms (P.moduleNameFromString "$Main") decls refs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If someone attempts to play with module export syntax this might be cause problems with:
module Foo (module Bar, module Foo) where ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll wager this is by far an edge case, however 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point. I think re-exporting main
from elsewhere seems plausible to want to do, so perhaps this isn't the best approach. Maybe we should assemble a set of used module names from the externs files, and then instead of rewriting, just check that the given module name is not included in that set?
Seems to work. Fixes #174