fitzgen / inlinable_string

An owned, grow-able UTF-8 string that stores small strings inline and avoids heap-allocation.
http://fitzgen.github.io/inlinable_string/inlinable_string/index.html
Other
69 stars 11 forks source link

Missing FromStr impl for InlinableString #29

Closed hniksic closed 3 years ago

hniksic commented 3 years ago

Please consider adding a trivial FromStr impl for InlinableString.

As far as I can tell, InlinableString doesn't implement FromStr. While it seems silly at first to depend on FromStr when one can just call InlinableString::from, it can come useful for generic code, and String provides one. For example, we have code like this:

let foo = Foo {
    option_u8_field: parse_non_empty(csv_field_1)
    option_string_field: parse_non_empty(csv_field_2),
    // ... many other fields ...
}

parse_non_empty is defined for all T: FromStr, and returns aResult<Option<T>>, immediately returning Ok(None) for empty string, and returning Some(T::from_str(s)?) otherwise. It's elegant to be able to use the same empty-string logic when T is String as when T is a number or a custom type with FromStr impl.

The above code compiles for option:string_field: Option<String>, but not for option_string_field: Option<InlinableString> because InlinableString is not FromStr. Of course, I cannot implement FromStr for InlinableString due to orphan rules.

The current workaround is to replace parse_non_empty(csv_field_2) with if csv_field_2.is_empty() { None } else { Some(InlinableString::from(csv_field_2) }. The downside is that it's duplicating a part of the parse_non_empty logic - for example, if it were to change to strip the string before checking whether it's empty, it would have to be updated in two places.

ArtemGr commented 3 years ago

Bountysource