set_attribute: Taking ToString instead of AsRef<str>

[d4h0]

Hi,

Thanks for creating SilkenWeb – so far I really enjoy the nice macro-less API!

(It's smooth as silk... 😅)

While looking at the source code, I noticed the following function:

fn set_attribute(dom_element: &dom::Element, name: impl AsRef<str>, value: impl AsRef<str>) {
    clone!(dom_element);
    let name = name.as_ref().to_string();
    let value = value.as_ref().to_string();

    queue_update(move || dom_element.set_attribute(&name, &value).unwrap());
}

Basically, you:

1. request something that can be turned into &str
2. convert it (potentially a String) into a &str
3. and then you turn that &str into a String.

That doesn't seem to make sense to me – is there a reason for that?

I believe, ToString would be the right trait to request:

fn foo<T: ToString>(s: T) {
    println!("{}", s.to_string())
}

fn main() {
    foo("bar");
    foo("bar".to_string())
}

[simon-bourne]

Hi, glad you like it!

I agree, impl AsRef<str> doesn't look right, and there are probably other places in the code like this. I think Into<String> might be the right trait to require here, but I'll take a look. ToString is implemented in terms of Display and specialized for some std types.

[d4h0]

Oh, right, Into<String> might be better.

[simon-bourne]

set_attribute now takes string arguments as Into<String>