Skip to content

Conversation

@artursvonda
Copy link
Contributor

@artursvonda artursvonda commented Dec 5, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? no
Tests pass? yes (relevant)
Fixed tickets 415
License Apache2

Description

Considers choice translation domain when extracting translation strings. Also ignores translates that have false as translation domain.

Todos

  • Tests
  • Documentation
  • Changelog

@artursvonda
Copy link
Contributor Author

Wasn't sure if this qualifies as BC break since extractor previously didn't take into account choice_translation_domain which will cause translations to be extracted under different (correct) domain now. Also will correctly skip translations with false translation domain.

@artursvonda artursvonda force-pushed the use-choice-translation-domain branch from de3b1f7 to 7dce4f0 Compare December 5, 2016 12:54
@electrixx90
Copy link

When it becomes available this feature? I hope soon...

@gnat42
Copy link
Collaborator

gnat42 commented Apr 20, 2017

the other issue with this change is that the choice_translation_domain doesn't exist as a valid option for sf 2.3 which for whatever reason this bundle still supports even though 2.3 is no longer supported. We would have to drop support for sf2.3 with this.

@artursvonda
Copy link
Contributor Author

I don't think we need to drop the support for 2.3. The translation domain falls back to previous behaviour of using translation_domain if choice domain is not available. That is backwards compatible.

@gnat42
Copy link
Collaborator

gnat42 commented Apr 20, 2017

Well the tests will be trying to pass an option that doesn't exist in 2.3. So if we don't drop 2.3 support we need to then only conditionally run those tests when sf is > 2.3.

@artursvonda
Copy link
Contributor Author

I don't think test forms are actually executed so it shouldn't matter. And if users try to use the option on the forms, Symfony will inform them that the option doesn't exist.

@gnat42
Copy link
Collaborator

gnat42 commented Apr 20, 2017

Can you rebase this on master please?

@artursvonda artursvonda force-pushed the use-choice-translation-domain branch from 7dce4f0 to fdc8f32 Compare April 20, 2017 18:47
@artursvonda
Copy link
Contributor Author

Done :)

@gnat42
Copy link
Collaborator

gnat42 commented Apr 20, 2017

Can you review the test failures and see if you can fix/determine the issue?

@artursvonda artursvonda force-pushed the use-choice-translation-domain branch from fdc8f32 to c2e3854 Compare December 7, 2018 12:50
@artursvonda
Copy link
Contributor Author

Updated tests and rebased. No idea why that one test is failing though.

@yellow1912
Copy link

This is a very old pull request but it has some important improvements that can be considered. For example, it seems like in the current version the translation_domain and choice_translation_domain when set to false still trigger the extraction. This is in conflict with Symfony's document which clearly states that false mean no translation.

@artursvonda artursvonda force-pushed the use-choice-translation-domain branch from 5ff05e2 to 2aa7af9 Compare April 11, 2021 12:47
@artursvonda
Copy link
Contributor Author

Rebased against latest develop

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.

4 participants