search

rhaiscript / rhai

Rhai - An embedded scripting language for Rust.
https://crates.io/crates/rhai
Apache License 2.0
3.77k stars 178 forks source link

Add ref_* methods for Dynamic. #902

Closed madonuko closed 1 month ago

madonuko commented 2 months ago

These methods does not take ownership and are easier to work with than their into_* counterparts.

schungx commented 2 months ago

Can these be replaced by lock_read?

schungx commented 2 months ago

Come to think of it and experimenting with a number of different names, I suggest as_XXX_ref and as_XXX_ref_mut in order to avoid conflicts with a deprecated API called as_immutable_string.

I have actually put together a PR here: https://github.com/rhaiscript/rhai/pull/904

Would you wait until I commit this, then rebase your further changes on top of it?

madonuko commented 2 months ago

Sure, I'm happy to wait. Also, why not as_XXX_mut (following rust std)?

schungx commented 2 months ago

Sure, I'm happy to wait. Also, why not as_XXX_mut (following rust std)?

Hhhmmm... Good idea. I'll change.

schungx commented 2 months ago

Changed in https://github.com/rhaiscript/rhai/pull/905

schungx commented 2 months ago

Is it necessary to keep the get_XXX versions since the only difference now is Option vs Result...

madonuko commented 2 months ago

Well I guess the compiler can optimize the error handling away while compiling this crate?

schungx commented 2 months ago

Well I guess the compiler can optimize the error handling away while compiling this crate?

Yes it can... but I'm not sure having get_string() would be significant better than as_immutable_string_ref().ok()... it is quite a bit shorter, true, but also very similar in API...

For others, get_array() and as_array_ref().ok() are not as pronounced in the length of the call...

schungx commented 1 month ago

I would close this for the time being until later date when more API needs to be added.