Skip to content
This repository was archived by the owner on Mar 11, 2025. It is now read-only.

I18n - #700

Closed
AndromedaCLI wants to merge 70 commits into
askama-rs:mainfrom
AndromedaCLI:i18n
Closed

I18n#700
AndromedaCLI wants to merge 70 commits into
askama-rs:mainfrom
AndromedaCLI:i18n

Conversation

@AndromedaCLI

Copy link
Copy Markdown

This PR is based on the work of jhoobergs and PR #434.
I mainly just updated the code to work with the changes that have been made since the initial PR, and I split it into its own feature "localization" which can be renamed to maybe "with-localization".
Also I added askama::Locale as suggested in #434.

Pending:

  • Docs
  • Improve the feature, mainly the use of #[cfg(feature = "localization")] since I'm new to rust/cargo

All tests pass.
Looking at the old PR the TODO: "Fix the test for invalid text_id's" isn't really needed since we make use of a function from fluent that checks for a fallback language which is why the test doesn't panic, in fact it will never panic as seen here

Comment thread askama_derive/src/parser.rs Outdated
Comment thread askama_derive/src/input.rs Outdated
Comment thread askama_derive/src/input.rs Outdated
Comment thread askama_derive/src/parser.rs
Comment thread askama_derive/src/parser.rs Outdated
Comment thread askama_derive/src/generator.rs Outdated
@LeoniePhiline

LeoniePhiline commented Oct 11, 2022

Copy link
Copy Markdown

CI fails due to a newly introduced match at Add Expr::is_cachable().
The i18n branch is currently outdated and must be rebased.

@11Tuvork28 Could you rebase your branch on top of the current main? I'll then mark Expr::Localize(_, _) as cacheable via recursive arguments check.

AndromedaCLI and others added 6 commits October 11, 2022 17:14
docs: Rename example template struct
Quote from https://projectfluent.org/fluent/guide/hello.html

```ftl
hello = Hello, world!
```

Each message has an identifier that allows the developer to bind it to the place in the software where it will be used. The above message is called `hello`.
fix: Implement missing recursive `is_cachable()` for `Expr::Localize`
@LeoniePhiline

Copy link
Copy Markdown

I implemented missing recursive is_cachable() for Expr::Localize after the fork was updated.

Would a maintainer please be so kind and approve the workflow? Thank you!

@LeoniePhiline

LeoniePhiline commented Oct 11, 2022

Copy link
Copy Markdown

Please excuse, my mistake. I had missed to check compilation with default features (without i18n).

This is fixed now in AndromedaCLI#7, waiting to be merged into this PR branch.

Once AndromedaCLI@2bbba70 has landed in this PR, the checks should succeed.

Edit: Landed - all checks pass now.

Fix: Add missing trailing newline in ftl file
@djc

djc commented Oct 11, 2022

Copy link
Copy Markdown
Collaborator

@LeoniePhiline and @11Tuvork28 thanks for your patience with me and for the hard work on getting this ready. I'll try to make another pass soon, but feel obliged to warn you that I have a very busy week and generally a lot on my plate.

@TTWNO

TTWNO commented Apr 17, 2023

Copy link
Copy Markdown

I'd like to pop in on this issue and ask if anything needs to be done to get this merged?

I'm writing a website for my hockey league that needs pretty advanced i18n that fluent provides; I have some time, so please let me know what needs to be done.

@AndromedaCLI

Copy link
Copy Markdown
Author

I'd like to pop in on this issue and ask if anything needs to be done to get this merged?

I'm writing a website for my hockey league that needs pretty advanced i18n that fluent provides; I have some time, so please let me know what needs to be done.

Rebase for one would be good, then I'm not sure that this is even wanted anymore given that this hasn't gotten any anttention.

@djc

djc commented Apr 20, 2023

Copy link
Copy Markdown
Collaborator

Again, sorry to everyone following this PR for my lack of feedback here. Please be assured that I think about this PR at least once a week, but in its current state it's hard for me to justify taking out the time to review the large amount of changes here, compared to a bunch of other stuff (roughly across my family, my day job and other open source projects that I help maintain).

I guess the main thing that could help resolving this deadlock would be to represent the changes here as a series of a smaller PR that have one logical change per PR, so that I can review each of them quickly and thus we can make progress. I should probably have given this feedback sooner, but I figured I'd be able to get to it sooner.

I hope that someone is willing to take that one -- I really would like to get this work merged -- and thanks to everyone who has worked on it so far.

@AndromedaCLI

Copy link
Copy Markdown
Author

Thanks for the feedback!!
Much appreciated, I will try to take some time and break down the changes into logical blocks.
And no worries about not getting to review stuff, its understandable we all have only 24 hours per day.

@TTWNO

TTWNO commented Apr 20, 2023

Copy link
Copy Markdown

If you want to see a recently-rebased version you can take a look at my fork.

@AndromedaCLI

Copy link
Copy Markdown
Author

If you want to see a recently-rebased version you can take a look at my fork.

Can you PR that to my fork? Would be sweet :)
But I will take a look regardless, maybe we could work together on splitting it up? Would make it fast and more doable for me

@djc

djc commented Apr 20, 2023

Copy link
Copy Markdown
Collaborator

You can't really PR a rebase. However, as the PR author you can reset to the rebased commit/branch.

@TTWNO TTWNO mentioned this pull request Jul 16, 2023
@mgmuesuu

This comment was marked as spam.

@AndromedaCLI

Copy link
Copy Markdown
Author

Closing in favor of #841 and its related PRs.
Thanks again to everyone involved.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants