Split stdlib into submodules #152
Conversation
|
We have to update this discussion once this gets merged and released |
src/compiler.rs
Outdated
| std_modules.insert("".to_string(), include_str!("std/main.ab").to_string()); | ||
| std_modules.insert("std".to_string(), include_str!("std/main.ab").to_string()); | ||
| std_modules.insert("strings".to_string(), include_str!("std/strings.ab").to_string()); | ||
| std_modules.insert("fs".to_string(), include_str!("std/fs.ab").to_string()); |
There was a problem hiding this comment.
Is there any problem with this namespace definition?
I think it will affect future language design.
For example, I feel std/string is better than std/strings.
There was a problem hiding this comment.
I'm also struggling to understand why just the "std" isn't good enough. Is there any deep reason behind this since if we just import functions separately and they got tree-shaken, @b1ek?
There was a problem hiding this comment.
I'm also struggling to understand why just the
"std"isn't good enough
as of right now, its fine as it is but when we'd want to grow it to have an imagick, curl, fs and other kind of modules it will be a mess if they are placed in one file. i have actually started working on this PR because i wanted to make a curl module but didn't want to place it in main.rs
i mean, you dont do stuff like use std::{read_file_to_string, SocketAddr, IoError} in rust, right? even if unused stuff will get tree shaken away
There was a problem hiding this comment.
I see. Why not import everything publicly from the main.ab and have string.ab and other modules?
pub import * from "./string.ab"
Oh... this could be hard to implement for the path resolution to work. I see now.
There was a problem hiding this comment.
Why not import everything publicly from the
main.ab
thats actually the right way. we probably should either load stdlib from place like /usr/lib/amber/std or look into the include_dir macro
There was a problem hiding this comment.
Yeah, this would require the installer to fetch the stdlib as well however this would make this issue #170 impossible to implement. I think that for now we could leave the current state of the PR:
import * from "std/string"I think that we should also discuss how the paths should work with package manager in the future. Here are some ideas I had in mind:
Not sure if we want to get our hands dirty and maintain package registry. This would require some funding and good sotware architecture as it'd be easy to abuse it. If we decide to keep things simple - we could offer importing packages directly from git repositories as the main way to fetch packages.
// Pull the code from GitHub repository (latest)
import * from "gh:b1ek/amber-library"
// Pull the code from GitHub repository with version
import * from "gh:b1ek/[email protected]"
// Pull the code from url (the version should be dependent on the url)
import * from "https://example.com/amber-library"
// Import the code from path
import * from "./path/to/file"This would require us to implement an amber.lock file to keep the track on versioning of all dependencies.
There was a problem hiding this comment.
this would require the installer to fetch the stdlib
why? the stdlib exists only at compile time. there's no need to source it in the script at the runtime.
as for package registry, that is probably the job for a third party cli application, as its been done for node, rust, and many other languages
There was a problem hiding this comment.
I wanted to point out that Deno did it in the early stage but now I see that they discourage it as people can easily put malware in the third party url.
Never mind. This was not a good idea.
I think you have a good approach here.
src/compiler.rs
Outdated
| pub fn resolve_std(path: String, meta: &mut ParserMetadata, token_import: Option<Token>) -> Result<String, Failure> { | ||
| let mut std_modules = HashMap::new(); | ||
|
|
||
| std_modules.insert("".to_string(), include_str!("std/main.ab").to_string()); |
There was a problem hiding this comment.
I don't like this approach to let people import from:
import * from "std"
// or
import * from "std/"As this would result in inconsistencies in path naming. I'd just error that std/ doesn't exist. What do you think?
There was a problem hiding this comment.
@b1ek can you rename this module to text.ab?
|
I think that this PR depends on #266 When that one will be merged we can rewrite this one and reorganize everything. |
|
#266 is merged and we can now update this PR |
|
If it is fine for @b1ek I can work on a new version of this PR with the latest changes to the stdlib, as the Rust code doesn't seems so much to handle. |
|
@Mte90 if you want to make it mergeable, you can just push commits to this branch to not do extra work |
|
There is so much going on with stdlib PRs. Let's prioritize this one first |
|
Okay I'm closing this PR in favour of #291 |
reworked a little bit of stdlib. the whole thing is still in an early stage, but now it can have more than one file
stating the obvious, this is a breaking change by definition.
issues:
hardcoded paths. i do not know macro-fu that much to dynamically load the modulesinstall script needs rewriting. working on that right nowthis PR also introduces
include_dircrate dependency