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 Fluent function to Tera, link to data context #21

Merged
merged 8 commits into from
Jun 23, 2023

Conversation

alerque
Copy link
Contributor

@alerque alerque commented Feb 16, 2023

Closes #20.

I'm not super happy with the ergonomics here. Part of the problem turned out to be the fluent-template crates resource loader wasn't quite as robust as hoped. Second there doesn't seem to be an easy way to make it cooperate with locales that don't have resource directories. We can fallback to a single FTL resource, but a directory still has to exist for any locales one attempts to use. Hence I didn't even include a way to pass a single file yet. I have an upstream issue open about that and am hoping to come up with a way to make this usable with just passing a single FTL file like one might pass a specific context file.

I don't think this will break any current usage at all.

@alerque
Copy link
Contributor Author

alerque commented Feb 17, 2023

CI failures were because it was not being tested with features enabled, but it should be tested both with and without. I added the flag for --all-features, but I think the matrix needs to be extended to cover both cases.

@alerque alerque marked this pull request as draft February 17, 2023 11:27
@alerque alerque marked this pull request as ready for review February 17, 2023 11:33
@alerque
Copy link
Contributor Author

alerque commented Feb 17, 2023

I expanded the test matrix to run the cargo test and check functions both with and without feature flags. I made the clippy one run just with all features because I don't think that will ever turn up more useful results with less features enabled. I also fixed an issue with the code when the feature is not enabled.

@alerque
Copy link
Contributor Author

alerque commented Apr 22, 2023

Any chance on some feedback on this @chevdor? I'm already using a custom build for some work that needs it, but keeping that up to date is a bit obnoxious, especially considering now I have to bump all the systems that got upgraded to the now-newer release version.

If you don't plan on accepting this at least let me know so I can fork and move on. If you are still interested let me know what it needs to get merged and in a release.

@chevdor
Copy link
Owner

chevdor commented Jun 23, 2023

@alerque sorry for the time it took. Your update just landed when I took some time off and it then slipped out of my lists :)

Copy link
Owner

@chevdor chevdor left a comment

Choose a reason for hiding this comment

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

fast-fail => fail-fast should work better :)

.github/workflows/quick-check.yml Outdated Show resolved Hide resolved
.github/workflows/quick-check.yml Outdated Show resolved Hide resolved
@alerque
Copy link
Contributor Author

alerque commented Jun 23, 2023

So sorry about the CI issues. I was wondering how I missed that, but the main reason is that it seems to be disabled on PRs except when they are first opened, and the CI changes were not added until later. You might consider dropping the types: limitation on the pull_request: trigger. Just a thought for later.

@chevdor
Copy link
Owner

chevdor commented Jun 23, 2023

This is due to the fact that I need to approve the runs so they don't run immediately but they do.
It would be best/faster to test locally in your repo to ensure we have all the fixes. Some more fixes are required: you cannot pack command and args on a line.... so you need to split the 2.

@chevdor chevdor merged commit 2cc3074 into chevdor:master Jun 23, 2023
@alerque alerque deleted the fluent branch September 12, 2023 10:59
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.

Add Fluent language functions
2 participants