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.

Sign up for GitHub

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

Jump to bottom

Runtime translation detection #69

Merged
merged 3 commits into from
Apr 13, 2023
Merged

Runtime translation detection #69

merged 3 commits into from
Apr 13, 2023

Conversation

m-horky
Copy link
Contributor

@m-horky m-horky commented Apr 4, 2023

Hello. Thanks for creating gettext-compatible Go package, it is much more elegant than the existing alternatives!

After reading #42, I decided to take a look into the package. The original rejected PR #41 already had the core ideas, which I adopted into a new design.

Since the originally suggested change would break the API compatibility, I decided to take another round and implemented IsTranslated* methods that can be used to detect whether a string has translation or not. They bubble up from individual Translations up into the public module methods.

The patch is split into three files. The two big changes contain their own tests. This patch should not be able to change any behavior, it does not alter the internal logic at all.

Is this a fix, improvement or something else?

This PR is an improvement.

What does this change implement/fix?

This PR implements methods to allow runtime detection of translations.

I have ...

  • answered the 2 questions above,
  • discussed this change in an issue,
  • included tests to cover this changes.

@m-horky
Rename contexts to contextTranslations
4ce6b50 
This will keep the names consistent across the three members
@m-horky
Detect translations in low-level structs
8941f8b 
This patch adds .IsTranslated(), .IsTranslatedN() and related functions
to following objects:
- Translation
- Domain

This makes it possible to detect whether a string is translatable or not
during runtime.
@leonelquinteros
Copy link
Owner

Hi @m-horky , thanks for your contribution, I like the approach for the implementation.

That said, I'm not so sure about the change in the Translator interface. I wouldn't add these methods there for compatibility reasons, any other users upgrading the lib will have to add these methods to their implementations of this interface.

Would you create a new interface for this methods? I'd say a small interface (recommended and idiomatic I think) that only contain this methods.

What do you think?

@m-horky
Copy link
Contributor Author

m-horky commented Apr 5, 2023

Hi, good idea, I'll some up with something and get back :)

@m-horky m-horky marked this pull request as draft April 5, 2023 11:12
@m-horky
Detect translations in high-level structs
3a68971 
This patch adds .IsTranslated(), .IsTranslatedN() and related functions
to following objects:
- Po
- Mo
- locale
- gotext
and it creates helper interfaces in introspector.go.

This makes it possible to detect whether a string is translatable or not
during runtime.

Resolves leonelquinteros#42
@m-horky m-horky marked this pull request as ready for review April 12, 2023 15:17
@m-horky
Copy link
Contributor Author

m-horky commented Apr 12, 2023

Hi. Let me know if this variant is any better and if you have more notes!

@m-horky m-horky mentioned this pull request Apr 12, 2023
Closed
@leonelquinteros leonelquinteros merged commit 4829902 into leonelquinteros:master Apr 13, 2023
@leonelquinteros
Copy link
Owner

Looking good now, thanks for your contribution!

@m-horky m-horky deleted the is-translated branch April 13, 2023 13:47
nono added a commit to cozy/cozy-stack that referenced this pull request Apr 15, 2024
@nono
fix(deps): update module github.com/leonelquinteros/gotext to v1.6.0 (#…
3f4c8ed 
…4376)

[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[github.com/leonelquinteros/gotext](https://github.com/leonelquinteros/gotext)
| `v1.5.2` -> `v1.6.0` |
[![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2fleonelquinteros%2fgotext/v1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/go/github.com%2fleonelquinteros%2fgotext/v1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/go/github.com%2fleonelquinteros%2fgotext/v1.5.2/v1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2fleonelquinteros%2fgotext/v1.5.2/v1.6.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>leonelquinteros/gotext
(github.com/leonelquinteros/gotext)</summary>

###
[`v1.6.0`](https://github.com/leonelquinteros/gotext/releases/tag/v1.6.0)

[Compare
Source](https://github.com/leonelquinteros/gotext/compare/v1.5.2...v1.6.0)

#### What's Changed

- Allow locale to use a `fs.FS` by
[@&#8203;kobergj](https://github.com/kobergj) in
[leonelquinteros/gotext#68
- Runtime translation detection by
[@&#8203;m-horky](https://github.com/m-horky) in
[leonelquinteros/gotext#69
- feat: Add language getter for Locale by
[@&#8203;taozhou-glean](https://github.com/taozhou-glean) in
[leonelquinteros/gotext#77
- Allow setting global storage by
[@&#8203;didrocks](https://github.com/didrocks) in
[leonelquinteros/gotext#80
- Fix multiline export by
[@&#8203;didrocks](https://github.com/didrocks) in
[leonelquinteros/gotext#82
- Order locations in ascii order by
[@&#8203;didrocks](https://github.com/didrocks) in
[leonelquinteros/gotext#83
- Fix io/fs File leak by
[@&#8203;diamondburned](https://github.com/diamondburned) in
[leonelquinteros/gotext#79
- Add support for multiple languages by
[@&#8203;m-horky](https://github.com/m-horky) in
[leonelquinteros/gotext#73
- Revert "Add support for multiple languages" by
[@&#8203;leonelquinteros](https://github.com/leonelquinteros) in
[leonelquinteros/gotext#84
- Remove unused dependency by
[@&#8203;der-lyse](https://github.com/der-lyse) in
[leonelquinteros/gotext#86
- Fix typo in documentation by
[@&#8203;der-lyse](https://github.com/der-lyse) in
[leonelquinteros/gotext#87
- Add support for multiple languages by
[@&#8203;m-horky](https://github.com/m-horky) in
[leonelquinteros/gotext#85

#### New Contributors

- [@&#8203;kobergj](https://github.com/kobergj) made their first
contribution in
[leonelquinteros/gotext#68
- [@&#8203;m-horky](https://github.com/m-horky) made their first
contribution in
[leonelquinteros/gotext#69
- [@&#8203;taozhou-glean](https://github.com/taozhou-glean) made their
first contribution in
[leonelquinteros/gotext#77
- [@&#8203;didrocks](https://github.com/didrocks) made their first
contribution in
[leonelquinteros/gotext#80
- [@&#8203;diamondburned](https://github.com/diamondburned) made their
first contribution in
[leonelquinteros/gotext#79
- [@&#8203;der-lyse](https://github.com/der-lyse) made their first
contribution in
[leonelquinteros/gotext#86

**Full Changelog**:
leonelquinteros/gotext@v1.5.2...v1.6.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 6am on Monday" in timezone
Europe/Paris, Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/cozy/cozy-stack).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjI5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@m-horky @leonelquinteros