-
Notifications
You must be signed in to change notification settings - Fork 160
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
feat: add support for nested modules #992
Conversation
cf7cfc9
to
637e1ea
Compare
352d426
to
4441f27
Compare
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.
Thank you! Looks good! I left some comments inline. Basically, I'm thinking we can change the approach so that all path manipulations are done at the time when we build libraries and everything else works as is. Basically, as we traverse directory tree, if we see something like x/y/mod.masm
we transform this into library path x::y
.
We should also probably add a check to the library construction logic to make sure that we get an error if there a file with the same name as the directory. For example:
/x/y/mod.masm
/x/y.masm
The above should fail because having y.masm
conflicts with y/mod.masm
.
4441f27
to
05fd796
Compare
05fd796
to
d9015bd
Compare
4254f1a
to
c39bb46
Compare
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.
Looks good! Thank you! I left a couple of small comments inline.
One thing I'm wondering is whether there is a good way to test this somehow. Maybe we could add some useful function under std::math
or something like that?
One option is to re-export methods of the |
Good idea! Let's do this: re-export |
c39bb46
to
5e520cd
Compare
5e520cd
to
89a918d
Compare
One other question is if we should add a build step to copy the docs from |
No, the formatting on auto-generated docs needs to be improved quite a bit. |
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.
All looks good! Thank you!
Describe your changes
Addressing #983
Checklist before requesting a review
next
according to naming convention.