cargo-i18n icon indicating copy to clipboard operation
cargo-i18n copied to clipboard

use fl! macro to pass a string slice to a function

Open wiiznokes opened this issue 1 year ago • 7 comments

I need to use the String generated from fl! macro to be used in a fonction that take &str as parameter.

let error_text = fl!("already_use");
name.error(&error_text);

This is the error i get:

error[E0308]: mismatched types
  --> ui/src/localize.rs:26:9
   |
26 |         i18n_embed_fl::fl!($crate::localize::LANGUAGE_LOADER, $message_id)
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `&str`, found `String`
   |
  ::: ui/src/item.rs:94:27
   |
94 |         name = name.error(fl!("already_use"));
   |                           ------------------ in this macro invocation
   |
   = note: this error originates in the macro `fl` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider borrowing here
   |
26 |         &i18n_embed_fl::fl!($crate::localize::LANGUAGE_LOADER, $message_id)
   |         +

For more information about this error, try `rustc --explain E0308`.
error: could not compile `ui` (lib test) due to previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `ui` (lib) due to previous error
make: *** [Makefile:22 : fix] Erreur 101

wiiznokes avatar Dec 09 '23 01:12 wiiznokes

I imagine that to be able to do this, we would have to generate all the strings in a static array, and that fl! return a &'static str.

Any plan or thought on that ?

wiiznokes avatar Dec 09 '23 01:12 wiiznokes

Hi @wiiznokes this is slightly related to #109

It looks like the code in your example is different to the one referenced in the error message?

let error_text = fl!("already_use");
name.error(&error_text);
  ::: ui/src/item.rs:94:27
   |
94 |         name = name.error(fl!("already_use"));
   |                           ------------------ in this macro invocation
   |

I think it should work what you have provided in the example, but to be sure, what is the type signature of .error() function? Another thing you can try is the .as_str() method on String.

kellpossible avatar Dec 11 '23 16:12 kellpossible

I imagine that to be able to do this, we would have to generate all the strings in a static array, and that fl! return a &'static str.

Any plan or thought on that ?

The error message doesn't mention anything about lifetimes, does .error() require a &'static str ?

kellpossible avatar Dec 11 '23 16:12 kellpossible

I have the impression that the error message I provided was not the correct one. I get it with this code:

name = name.error(fl!("already_used_error"));

Which is obviously not good.

With this code:

let error_text = fl!("already_used_error");
name = name.error(&error_text);

this is the error message:

 cargo run
   Compiling ui v0.1.0 (C:\Users\wiiz\Documents\fan-control\ui)
error[E0515]: cannot return value referencing local variable `error_text`
   --> ui\src\headers.rs:110:5
    |
64  |         name = name.error(&error_text);
    |                           ----------- `error_text` is borrowed here
...
110 |     elems
    |     ^^^^^ returns a value referencing data owned by the current function

For more information about this error, try `rustc --explain E0515`.
error: could not compile `ui` (lib) due to previous error

I think the error would disappear if I stored the string in my app state, but that wouldn't be intuitive, which is why I would have preferred an api that directly returns a reference to me.

Also, rust analyser seems confused with the return type of the macro: image

wiiznokes avatar Dec 29 '23 17:12 wiiznokes

I think that the problem here is the ownership of the message returned by the fl!() macro, for instances where messages are being formatted with dynamic variables, it will need to be owned in the local variable. Ideally we would probably have the output of fl!() be Cow<str> but we still have the problem that there is no way to differentiate between dynamically formatted messages and those that are simply references to messages stored statically in the program. Perhaps there needs to be an alternate version of fl!() to support this use case. Or alternatively, you can change the signature of name.error() to take ownership of the error text, so that when you return it, it is not holding on to a reference to a local variable.

kellpossible avatar Jan 18 '24 14:01 kellpossible

Just out of interest, what is the type signature for the function name.error()? @wiiznokes

kellpossible avatar Jan 18 '24 14:01 kellpossible

Just out of interest, what is the type signature for the function name.error()?

/// Sets the error message of the [`TextInput`].
    pub fn error(mut self, error: &'a str) -> Self {
        self.error = Some(error);
        self
    }

It comes from this file: https://github.com/pop-os/libcosmic/blob/efe4ce2f5b514e4d553ab82c0c873dca7585c028/src/widget/text_input/input.rs#L267

wiiznokes avatar Jan 20 '24 13:01 wiiznokes

The lifetime for the messages stored in FluentLoader makes it difficult to return a reference to them, even if there is no message formatting going on. This is because it's possible to unload the current bundles without destroying the loader itself. For now I'm pushing a breaking change in v0.8.0 to i18n-embed-fl which fixes #109 and ensures fl!() always returns String as was the original intention. With this change, if it's possible for you to store an owned value for error instead of &str this should hopefully resolve your problem?

I recognise that only returning String can result in a lot of extra allocations if your application primarily relies on static strings (which I guess to be realistic is most applications). We could perhaps modify the return value of fl!() to be a struct that contains a smart pointer to the bundle from which the message is contained, and some kind of self reference to that pointer for the message itself, but this is quite a bit of extra work that I don't have time to do at the moment and makes the API and code more complicated.

kellpossible avatar Feb 01 '24 08:02 kellpossible

Let me know if you thinks this issue should still be open, otherwise I think it might be resolved for now. We can open a new one if we need the feature for returning references to messages within the loader.

kellpossible avatar Feb 01 '24 08:02 kellpossible