Follow up for #1833022: Only display interface language detection options to customize more granularity
Updated: Comment #0
Problem/Motivation
Reading language_types_set() is still a bit confusing.
Proposed resolution
Improve the comments, add a has_default_settings variable
Remaining tasks
- get feedback
- get tests to pass
User interface changes
No.
API changes
No.
Related Issues
Comment | File | Size | Author |
---|---|---|---|
#9 | language_types_set-2033983-9.patch | 3.7 KB | balagan |
#6 | language_types_set-2033983-6.patch | 3.93 KB | balagan |
#1 | language_types_set-2033983-1.patch | 4.7 KB | YesCT |
Comments
Comment #1
YesCT CreditAttribution: YesCT commentedWith ideas from @plach @e0ipso and @kfritsche and lots of discussion about what the purpose is and how it works.
Comment #2
YesCT CreditAttribution: YesCT commentedActually, I think this ! was a bug. And that's why there are changes to the tests.
Locally, what seems like an unrelated test is failing because a page is now access denied and the test was expecting 404 page not found. (for a page with an unknown langcode prefix in the url).
LanguageUILanguageNegotiationTest around line 237
Comment #3
jhedstromComment #4
Gábor HojtsyUpdated the title based on where this code is now.
Comment #5
balagan CreditAttribution: balagan commentedComment #6
balagan CreditAttribution: balagan commentedI did not touch the /core/modules/language/lib/Drupal/language/Tests/LanguageNegotiationInfoTest.php file, it seems the changes in the patch in comment #1 are already there.
Comment #7
balagan CreditAttribution: balagan commentedComment #8
Gábor HojtsyShould be wrapped so as much is on one line as possible.
These lines are exceed 80 chars.
What does this mean? Not sure this comment clarifies the code? The later logic explains what happens when, not sure this lead-in helps.
Comment #9
balagan CreditAttribution: balagan commentedBroke the comments to be under 80 chars, and removed the ambiguous comment in 3. Not sure if I should write anything else, I have just copied text from patch in comment #1
Comment #10
balagan CreditAttribution: balagan commentedComment #11
Gábor HojtsyLooks good. I agree it is an improvement in some ways. Even if comments are too verbose here and there, it is clearly an improvement to me. Thanks!
Comment #12
alexpottI've spent time staring at and trying to grok this code - this is definitely better. Committed b971954 and pushed to 8.0.x. Thanks!
I've reflowed a couple of comments.
Comment #14
Gábor HojtsyYay, thanks all!