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

feat: support named arguments in localization #3181

Merged
merged 6 commits into from
Sep 11, 2023
Merged

Conversation

aatkin
Copy link
Collaborator

@aatkin aatkin commented Aug 29, 2023

relates #3183

  • add tempura support for map arguments with custom resource compiler rems.tempura/resource-compiler
  • use rems.tempura/tr as "base" function for rems.text (rems.tempura is shared code namespace)
  • removed rems.context/*tempura* with goal to share more code between clj/cljs

Localization strings may use either map or vector argument syntax, but not both. rems.tempura/resource-compiler can switch between map argument and vector arguments when both are provided (e.g. from rems.text/text-format):

(text-format :k {:arg1 :val1 :arg 2 :val2} :val1 :val2)
;;=> works for {:k "%1 %2"}
;;=> and {:k "%:arg1% %:arg2%"}

Custom resource compiler works as so:

  1. When translation fn is called, check provided arguments (vector).
  2. If arguments contain map as first element, check if resource (localization string) contains vector indexes ("%1" etc).
    2.1. If yes, drop first argument (rest arguments) and use resource as-is.
    2.2. If not, walk through resource and replace any keyword arguments "%:kw%" with equivalent vector index argument "%1". Use replaced resource string, and map argument map to ordered argument indexes (creating vector argument list): (mapv {:kw "%1"} [:kw]) ;=> ["%1"]
  3. Call tempura default resource compiler with returned resource and argument vector.

Checklist for author

Remove items that aren't applicable, check items that are done.

Reviewability

  • Link to issue

Documentation

  • Update changelog if necessary
  • Update docs/ (if applicable)
  • ADR for major architectural decisions or experiments (experiment for sure, but maybe not architectural decision?)

Testing

  • Complex logic is unit tested

Follow-up

  • New tasks are created for pending or remaining tasks (tasks maybe for updating email templates to use new translation keys?)

@aatkin aatkin force-pushed the tempura-map-args branch 2 times, most recently from d15dc35 to dfce476 Compare September 4, 2023 12:13
Tempura does not support map argument by default, but it can be configured using :resource-compiler. Here we re-use existing Tempura functionality by transforming named parameters into vector index parameters, then transform collected map parameters into sorted vector arg, and finally pass through Tempura default resource compiler. Map argument transformation is cached separately (from Tempura).
@aatkin aatkin marked this pull request as ready for review September 4, 2023 15:01
@aatkin aatkin requested a review from Macroz September 4, 2023 15:02
Copy link
Collaborator

@Macroz Macroz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can try this out.

@Macroz
Copy link
Collaborator

Macroz commented Sep 11, 2023

One thought for a follow-up task, we could try upgrading tempura since we haven't been able to, due to some backwards incompatibility.

@aatkin aatkin merged commit 98e5809 into master Sep 11, 2023
7 checks passed
@aatkin aatkin deleted the tempura-map-args branch September 11, 2023 21:16
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

2 participants