Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
language system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
14 Jan 2015 at 13:33 UTC
Updated:
17 Feb 2015 at 16:44 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
gábor hojtsyComment #2
gábor hojtsyComment #3
zealfire commented@Gabor, i was trying to solve this issue,but i have some doubts regarding implementation of filter languages().Since we have to remove setName() so do we need to override default language name as it was done by setName() ? I guess removing "$default->setName($this->t("Site's default language (@lang_name)", array('@lang_name' => $default->getName())));" this line would be fine.
Please tell me.Thanks.
Comment #4
gábor hojtsyThe idea is you would create a
new Language()with data appropriate for this display variant of the default language only for the runtime.Comment #5
sidharthapI am working on it.
Comment #6
develcuy commentedComment #7
develcuy commentedRemoved SprintWeekend2015Queue by mistake.
Comment #8
sidharthapComment #9
Cogax commentedComment #10
Cogax commentedI've made something, but i don't now if it's the solution. If not, please help me to get on the right way. Tanks!
Comment #11
Cogax commentedComment #13
Cogax commentedComment #14
Cogax commentedI updated the Testcases, so there was a test case for the setName method.
Comment #15
mglamanApplies, tests pass. Looks good.
Comment #16
alexpottThis is important - this allows sites to translate
Site's default language. We shouldn't be removing this. I think Gabor was thinking we should be creating new language object here.Comment #17
gábor hojtsyIndeed, looks good except:
Right, we want the label to be different here in the runtime. We should use
new Language()for a language we only use in the runtime.Comment #18
Cogax commentedThanks a lot, i hope i got it now. If not, please help me, i'ts all new to me :)
Comment #19
gábor hojtsyLooks good.
Comment #20
alexpottThis clarifies the difference between Language and ConfigurableLanguage and it also makes Language a true value object because it is immutable. Nice.
This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 574b0eb and pushed to 8.0.x. Thanks!
Comment #22
alexpottOops missed something - we should be moving LanguageInterface::setName to ConfigurableLanguageInterface since we are not removing ConfigLanguage::setName() - and we shouldn't be removing that.
Comment #24
gábor hojtsyYeah sorry I don't know how we missed that... Adding some test coverage also to ConfigurableLanguageUnitTest.
Comment #25
gábor hojtsyDuh patch did not include the changes in #24. Now does.
Comment #28
gábor hojtsyFails seems to be unrelated in Drupal.php where it gets an entity manager.
Comment #31
gábor hojtsyHm, related.
Comment #32
gábor hojtsy@Cogax: wanna continue with this one?
Comment #33
Cogax commentedSure, I'll try..
Comment #34
gábor hojtsy@Cogax: how is it going?
Comment #35
Cogax commentedI could need some help. I searched a lot and dived deeper and deeper into Drupal (that's the only good thing on that) but with no result.
The only thing that i think we should made is that we should implement a setUp() Method for the ConfigurableLanguageUnitTest. I've tried to setup there a Language Entity Manager (wich is needed in the label() Method of Entity.php) like it's been done in some other tests. But the Error still exists.
I had also problems to debug. I run the test alwas in my console with
but i don't know how to make test outputs like print_r / print / echo / var_dump etc. I've written this outputs always in a textfile wich isn't a good soloution..
Comment #36
Cogax commentedComment #37
vijaycs85As mentioned by @Cogax, we need the container to test setName/getName methods. Added new kernel test case which covers this.
Comment #38
gábor hojtsyYay, thanks, just some cleanup:
These style comments are not used on simpletest based tests, no?
Comment #39
vijaycs85Thanks for the review @Gábor Hojtsy. Here is an update...
Comment #40
gábor hojtsyLooks good, thanks!
Comment #41
alexpottThis issue is a normal bug fix, and includes only a very small disruptive change that contrib should not have been relying upon, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 4f776f6 and pushed to 8.0.x. Thanks!
Comment #43
gábor hojtsyThanks all!