Mapping Python print to JavaScript console.log #203
Conversation
|
I think creating a seperate Also, if you wish to replace the default print function with a new one, you can do so by setting the So far my opinions! |
vm/src/builtins.rs
Outdated
| /// Build the builtin module, | ||
| /// | ||
| /// * ctx: The Python Context | ||
| /// * builtin_overrides: A HashMap that contains (builtin item name, PyObjectRef to that item), |
There was a problem hiding this comment.
Instead of overrides, try to use the set_attr method to update a builtin function.
wasm/src/lib.rs
Outdated
| let mut overrides = HashMap::new(); | ||
| overrides.insert(String::from("print"), ctx.new_rustfunc(wasm_builtins::builtin_print)); | ||
| let builtins = builtins::make_module(&ctx, overrides); | ||
| let mut vm = VirtualMachine::new(ctx, builtins); |
There was a problem hiding this comment.
Use below here:
builtins.set_attr("print", ctx.new_rustfunc(wasm_builtins::builtin_print));|
Yes, your suggestion makes more sense. I'll work on it. |
|
@windelbouwman if we add a |
|
Btw, the following would be best in my opinion: let builtins = vm.import("builtins")?;
builtins.set_attr("print", ctx.new_rustfunc(javascript_print));This will work, since modules are cached once imported, so no copy of the namespace is made. So each piece of code using the builtin module refers to the same module. Then we do not need dependency injection into the vm during creation, and the VirtualMachine remains trivial to create! |
|
If you see this python documentation: file:///usr/share/doc/python/html/library/functions.html#print Notice that file has a default argument. Default arguments are already supported in rustpython, you can query for keyword arguments in rust code. Best way would be to leave the original print function, and implement |
|
Ah I didn't notice that |
|
That is correct, since |
wasm/src/lib.rs
Outdated
| //let ctx = vm.context(); | ||
| //let current_path = PathBuf::from("."); | ||
| //let builtins = import_module(&mut vm, current_path, "builtins").unwrap(); | ||
| builtins.set_attr("print", vm.context().new_rustfunc(wasm_builtins::builtin_print)); |
There was a problem hiding this comment.
@windelbouwman could you kindly help me check this part? It compiles correctly but seems to panic in runtime.
The browser gives this error:
bootstrap.js:5 Error importing `index.js`: RuntimeError: unreachable
at __rust_start_panic (wasm-function[3157]:1)
at rust_panic (wasm-function[3135]:31)
at std::panicking::rust_panic_with_hook::ha9514ccd859407a0 (wasm-function[3130]:305)
at std::panicking::continue_panic_fmt::hbdf64f9b91d66da7 (wasm-function[3129]:120)
at std::panicking::begin_panic_fmt::hb1f86670af92c2cf (wasm-function[3067]:95)
at _$LT$alloc..rc..Rc$LT$core..cell..RefCell$LT$rustpython_vm..pyobject..PyObject$GT$$GT$$u20$as$u20$rustpython_vm..pyobject..AttributeProtocol$GT$::set_attr::h29a4da0736316e29 (wasm-function[587]:367)
at rustpython_wasm::run_code::h6cdd6ee5f53d3532 (wasm-function[5]:344)
at run_code (wasm-function[6]:16)
at Module.run_code (webpack:///../pkg/rustpython_wasm.js?:34:76)
at eval (webpack:///./index.js?:5:57)
|
@windelbouwman I've pushed a few more commits to fix it. Please kindly review the final diff and I'll squash the commits before merge. |
@rmliddle @windelbouwman would you mind reviewing this code?
I'm not sure what's the best way to allow different version of
printbuiltin function. I add thebuiltinsargument to VM'snew(), so we can inject different version of builtin implementation to it. But maybe I need to use builder pattern or a Trait for that. What your thoughts on this?This is how it works, we can build more interesting webpage demo on top of it.