ISibboI / evalexpr

A powerful expression evaluation crate 🦀.
GNU Affero General Public License v3.0
325 stars 54 forks source link

Unexpected behaviour of the `str::from` builtin function #169

Closed VianneyRousset closed 1 month ago

VianneyRousset commented 1 month ago

The str::from builtin function is useful to convert value to a string, however the latter has an unexpected output when a string is given as argument.

"foo " + str::from(42) returns foo 42 which is fine. "foo " + str::from("bar") returns foo "bar", when it should be foo bar.

VianneyRousset commented 1 month ago

The str::from builtin function uses the Value::to_string implemented for the Display trait:

            Value::String(string) => write!(f, "\"{}\"", string),

The extra quotation marks above are the issue and does not follow the standard fmt() of a string. I guess that was a deliberate choice?

ISibboI commented 1 month ago

I see, thanks! Yeah, formatting a string with quotation marks was deliberate, but adding quotation marks around a string in str::from was not. I would be happy about a pull request that fixes this issue!

On Fri, 11 Oct 2024, 1.00 VianneyRousset, @.***> wrote:

The str::from builtin function uses the Value::to_string implemented for the Display trait:

        Value::String(string) => write!(f, "\"{}\"", string),

The extra quotation marks above are the issue and does not follow the standard fmt() of a string. I guess that was a deliberate choice?

— Reply to this email directly, view it on GitHub https://github.com/ISibboI/evalexpr/issues/169#issuecomment-2406115327, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASATXUJNLAGNJTD72DFDNLZ232IVAVCNFSM6AAAAABPXOPPGSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBWGEYTKMZSG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

VianneyRousset commented 1 month ago

Done in #170.

Please note the choice has been made that applying str:from to a tuple will still add those quotation marks to contained strings.

Therefore, str::from((42, true, "foo")) returns (42, true, "foo").

ISibboI commented 1 month ago

Thanks for fixing this! I'll hopefully be able to make a release during the next week.