Closed ltabis closed 4 months ago
Still too many _with_comments
for my comfort... I went down this route before and it took a lot of breaking changes and deprecation to finally move them all into the FuncRegistration
API... I would hesitate to add a whole bunch of _with_comments
yet again.
I am wondering if there is a way to do this without introducing a parallel set of API's...
How about if we include "the last function hash" inside TypeBuilder
. Then you can do:
builder
.with_get("foo", ...) // stores hash in builder type
.add_comments(...) // always uses hash in builder (if Some) to look up function in engine
....
The global namespace can be accessed via Engine::global_namespace_mut
. We can then add a crate-only method to Module
, say add_fn_comments
that can just lookup the function by hash, and then update the comments.
If you don't mind updating the parameter and return type names also, you can just use Module::update_metadata_with_comments
.
@schungx I think this is a good idea ... for functions. I guess that for types we still need a with_name_and_comments
function since the engine does not store hashes for them.
Since each builder has only one type, it is quite trivial to add a with_comments
for the builder.
Implementation wise, it can lookup the registered type via TypeId
and add comments to it.
This way no public API needs to change. Only a few more methods to the builder API.
I have done the changes, however when I execute the custom_types_and_methods
example the function parameters and return type information seems lost. I'm looking for the cause of the problem.
also rustc 1.77 seems to break the ui_tests/rhai_mod_inner_cfg_false.rs
test
I have done the changes, however when I execute the
custom_types_and_methods
example the function parameters and return type information seems lost. I'm looking for the cause of the problem.
Find out why: using the FuncRegistration
API directly does not set the params_info
by itself.
Fixed it by moving the information filling directly in FuncRegistration::register_into_engine
. (since it is called anyways in Engine::register_fn
)
Looking good!
A couple of observations:
Since an empty Vec
indicates no hashes, you can omit the Option
Probably not a good idea to move the default params info into register_into_engine
... What if the user used with_params_info
before? Those would be overwritten...
Maybe...
#[cfg(feature = "metadata")]
self = if self.metadata.params_info.is_none() {
...
self.with_params_info(...)
} else {
self
};
self......
A couple of observations
I agree with your comments, I made the changes.
I think it looks good. Let's merge this.
I wonder if it is possible to avoid adding update_fn_comments
...
Since the builder API is in the same crate, I think I can just inline the code in.
EDIT: I ended up replacing update_fn_comments
with the potentially more useful get_fn_metadata_mut
which allows you to change other fields.
Add documentation for types that derive the
CustomType
macro with themetadata
feature.