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

Enable date localization feature of tera #34

Merged
merged 2 commits into from
Feb 8, 2025
Merged

Conversation

alerque
Copy link
Contributor

@alerque alerque commented Feb 5, 2025

I spent a lot of time frustrated getting some templates right in a project a while back and ended up using something other than Tera (Handlebars) for it to get it out the door. I had to rework some other parts of it today and started fighting it again. The issue was a simple al outputting localized month names for dates in a table. First I tried generating localized versions of the context data with jq and various pre-processing and that was just a nightmare with a mess of files and non-deterministic array key sorting (it is easier to sort +%Y%m%d than +%B!). Then I tried post-processing the template after tera in the typesetting phase to match and replace things and that was spaghetti. Then I tried using the typesetter to localize based on the original date data, but it turns out convincing all the target environments to have the right system localization available is non-trivial.

In the end I finally ran across the Tera docs that note it can localize dates itself. Whew! This made the whole thing a breeze. Now I just have to use a binary built from my fork :(

This adds about 1.3MB (6.4→7.4 with fluent also enabled) to the runtime binary, but for my use case it is so worth it. I suspect others might find this useful too. I really don't understand having a minimal/hampered version of Tera available when the language can do such nice things.

Like the fluent feature I gated this behind a non-default feature flag assuming you'd prefer that, but if it were my project I would just make it a default-on feature. Let me know if you want that changed.

@chevdor
Copy link
Owner

chevdor commented Feb 7, 2025

I really don't understand having a minimal/hampered version of Tera available when the language can do such nice things.

I never needed/tested the extra features but I am sure it will come one day. There is no master plan to avoid extra features, however one big benefit of the minimal version is the reduced risks. I have nothing again adding extra flags. If a "standard" set of flags emerge from the "minimal", I am happy to keep the base minimalistic but add feature flags as you did and I am thinking that there may be a need for 2 binaries.

I don't think you use the binaries from the repo though, right ?

@chevdor
Copy link
Owner

chevdor commented Feb 7, 2025

If you wish, you can add a full feature and pack locale + fluent in.

@alerque
Copy link
Contributor Author

alerque commented Feb 7, 2025

I never needed/tested the extra features but I am sure it will come one day. There is no master plan to avoid extra features, however one big benefit of the minimal version is the reduced risks.

On principle there is some truth to that, but given that the features in question (e.g. a pure Rust built in date localization function that isn't even dependent on the host system) the "risk" is low and the potential UX is much better, I would suggest defaulting to enabling all the possible useful features and possibly supplying a minimal profile (--no-default-features?) for those that want a minimal footprint for whatever reason (extra work verifying an SBOM?).

I have nothing again adding extra flags. If a "standard" set of flags emerge from the "minimal", I am happy to keep the base minimalistic but add feature flags as you did and I am thinking that there may be a need for 2 binaries.

Yes, for convenience of people that use binaries I would suggest we could do a full build binary as well. Again my preference would be to default to full and have an optional secondary minimal binary, but as a contributor I'll just follow along with your decision for now.

I don't think you use the binaries from the repo though, right ?

Nope. I am an Arch Linux packager, and while the AUR package I maintain hasn't picked up enough votes or installs yet to warrant migrating it to official repositories, I do keep it packaged in a user repository signed with my official Arch Linux packager key so whether Arch forks build it from the AUR or use my built version the installation is pretty low friction.

It is quite typical for Arch to build Rust packages with cargo build --all-features unless there is some specific reason not to. The most common exception is when that enables self-updaters or telemetry or other anti-pattern features that Arch typically disavows.

If you wish, you can add a full feature and pack locale + fluent in.

Sounds good, I'll append that to this PR and also look at CI and see if an easy way to post a second binary jumps out at me.

@chevdor
Copy link
Owner

chevdor commented Feb 7, 2025

but as a contributor I'll just follow along with your decision for now.

Happy to discuss! I appreciate your feedback and arguments.

Security wise I am considering a few points, if we start pulling all features we expose all users, including those using tera-cli in CI to a huge deps tree with many feaures that may not be useful. Sure there is a security risk but as you mentioned, it remains rather small I agree. The features you included may however change and get dropped by the upstream projects (again the risk is rather low but who knows...). If we include them as default, the removal/change of such a feature may break usage for "standard" users and I would like to avoid that.

Atm, I would prefer to keep the base small but include your improvements to make it easy for users to get/make a full or customized version.

I am not really "warm to the idea of having CI build several binaries (ie will all mix and match of features) but it sounds quite reasonable to have default minimalistic version and the full version with all the features you introduced and that we consider stable enough.

Thatnks again for your work adding those !

@alerque
Copy link
Contributor Author

alerque commented Feb 7, 2025

Security wise I am considering a few points, if we start pulling all features we expose all users, including those using tera-cli in CI to a huge deps tree with many feaures that may not be useful. Sure there is a security risk but as you mentioned, it remains rather small I agree. The features you included may however change and get dropped by the upstream projects (again the risk is rather low but who knows...). If we include them as default, the removal/change of such a feature may break usage for "standard" users and I would like to avoid that.

Meh. We're not talking about adding a network-enabled feature or something, but whatever. Of course if we add a feature and then remove it that would be a breaking change, but from my perspective as a user of these optional features it already would be breaking if they stopped working, so my dog is already in the fight.

Atm, I would prefer to keep the base small but include your improvements to make it easy for users to get/make a full or customized version.

Added in 9201f6b.

I am not really "warm to the idea of having CI build several binaries (ie will all mix and match of features) but it sounds quite reasonable to have default minimalistic version and the full version with all the features you introduced and that we consider stable enough.

Of course adding a full matrix of options would be ridiculous, at most I would just bracket the two most common approaches: a minimal build and a full build. If people want to be selective in between they can build their own.

I looked at CI, and it makes less sense to me how to add this than I imagined. The binary artifacts being added now are not just the bare binaries but OS specific packages. For the distro-package use case I would (here I go again) recommend just doing an --all-features build.

The case where I think it could make sense to have options is bare binaries (that might get pulled and run in automated workflows like other CI runners). It looks like that is in #32 and not merged yet, so that PR would be the place to adjust the matrix of $CARCH and --feature flags that got built and posted.

@chevdor
Copy link
Owner

chevdor commented Feb 8, 2025

Yes my review on #32 is long due.

@chevdor chevdor merged commit 1cc2cdf into chevdor:master Feb 8, 2025
5 checks passed
@alerque alerque deleted the locale branch February 10, 2025 13:17
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.

2 participants