Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Portuguese and DynamicFormatter #23

Closed
wants to merge 8 commits into from
Closed

Add Portuguese and DynamicFormatter #23

wants to merge 8 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 1, 2021

This adds Português-BR (I'm native speaker). I'm grateful if you could publish it on crates.io.

I just added a DynamicFormatter too, which uses runtime Language. Now, Formatter wraps a DynamicFormatter.

@vi
Copy link
Owner

vi commented Mar 3, 2021

Does this bump MSRV? Currently it's 1.24.1.

@ghost ghost changed the title Add Portuguese Add Portuguese and DynamicFormatter Mar 3, 2021
@ghost
Copy link
Author

ghost commented Mar 3, 2021

Just bumped the MSRV.

@vi
Copy link
Owner

vi commented Mar 3, 2021

Published 0.2.2 with the new language, but without other changes.

Are you use the remaining changes are semver-compatible? Maybe they need 0.3.0?
Shall this dyn-clone be optional dependency?

@ghost
Copy link
Author

ghost commented Mar 3, 2021

The DynamicFormatter was going to break compatibility a bit. I'll end up forking it again. I was needing to use Formatter in my localization crate, however the problem is that timeago::Formatter structure is parameterized <L>, I need to hold a Formatter with any Language at runtime.

@vi
Copy link
Owner

vi commented Mar 3, 2021

my localization crate

Is it public? Maybe timeago can be integrated more closely into it?

the problem is that timeago::Formatter structure is parameterized , I need to hold a Formatter with any Language at runtime.

What's wrong with just Formatter<Box<Language>>?
Supplied executable example uses it to dynamically switch languages.

Cloning can be supplemented with just a minor change. Send+Sync - not sure, maybe with a version bump.

@ghost
Copy link
Author

ghost commented Mar 3, 2021

Thanks, I didn't know it could use a Box<dyn Language> as argument. Clone would still be needed if there were moves, but the Language structure is immutable, so I think it's good as is. I'll try using it in my crate.

@vi
Copy link
Owner

vi commented Mar 3, 2021

Pushed to master a version with impl Clone for Formatter<BoxedLanguage> (but not yet to crates.io).

Is this OK for you?

@ghost
Copy link
Author

ghost commented Mar 3, 2021

Using the current GitHub version gives this error:

struct S {
    // the trait bound `std::boxed::Box<(dyn timeago::Language + 'static)>: timeago::Language` is not satisfied
    // the following implementations were found:
    // <std::boxed::Box<(dyn timeago::Language + Send + Sync + 'static)> as timeago::Language>
    _current_timeago_formatter: Option<Rc<timeago::Formatter<Box<dyn timeago::Language>>>>,
}

It'd be complicated to add 'static to Box<dyn Language>. I'm guessing the dyn_clone dependency will still be necessary (as it keeps the trait safe).

@vi
Copy link
Owner

vi commented Mar 3, 2021

Is the code you are trying to build public, so that I can try to adapt it myself?

Do you use newly created type alias BoxedLanguage?

Do you have your own implementations of the Language trait (that are not included in the main timeago crate)?

@ghost
Copy link
Author

ghost commented Mar 3, 2021

Using BoxedLanguage works nice. Here's the crate that uses timeago: recoyx_localization.

@ghost
Copy link
Author

ghost commented Mar 3, 2021

Can you publish it again? Seems like it'll be backwards-compatible.

@vi
Copy link
Owner

vi commented Mar 3, 2021

as it keeps the trait safe

Was is a safe trait? I don't remember using any unsafe{} code in timeago.

@ghost
Copy link
Author

ghost commented Mar 3, 2021

I meant safe according to this Rust error.

@vi
Copy link
Owner

vi commented Mar 3, 2021

I meant safe according to this Rust error.

The correct term is "object safe". There is also unsafe trait ... {}, so just a "safe trait" could be confusing.

@vi
Copy link
Owner

vi commented Mar 3, 2021

Published v0.3.0

@ghost
Copy link
Author

ghost commented Mar 3, 2021

Thanks!

@ghost ghost closed this Mar 3, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant