Merge lp://qastaging/~mterry/unity-scopes-api/turkish into lp://qastaging/unity-scopes-api
Status: | Superseded |
---|---|
Proposed branch: | lp://qastaging/~mterry/unity-scopes-api/turkish |
Merge into: | lp://qastaging/unity-scopes-api |
Diff against target: |
106 lines (+20/-8) 7 files modified
debian/control (+1/-0) doc/tutorial.dox (+2/-1) src/scopes/internal/ConfigBase.cpp (+1/-1) src/scopes/internal/ScopeConfig.cpp (+3/-4) src/scopes/internal/Utils.cpp (+1/-1) test/gtest/scopes/internal/ScopeConfig/ScopeConfig_test.cpp (+1/-1) test/gtest/scopes/internal/Utils/Utils_test.cpp (+11/-0) |
To merge this branch: | bzr merge lp://qastaging/~mterry/unity-scopes-api/turkish |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Marcus Tomlinson (community) | Approve | ||
Review via email:
|
This proposal has been superseded by a proposal from 2016-10-17.
Commit message
Fix reading scope metadata in a Turkish locale.
Description of the change
I don't know of any existing bug symptom caused by this. I came across this while investigating a crash due to a misconfigured locale.
We're using std::locale("") to handle lowercasing user-input (well developer-input) scope metadata keys into standardized key names. This has two effects:
1) If the environment-
2) Using a locale-sensitive lowercasing algorithm has some odd side effects. Most notably in Turkish. See [1] and other such sites if you google "turkish i problem." Basically, if your problem domain is ASCII and/or you are not dealing with strings that the user will see, you probably don't want to be converting characters with the user's locale.
I've added a test that demonstrates the issue. With the old code, if you don't have the tr_TR.UTF-8 locale installed, the test will crash. If you do, it will fail. With the new code, it will pass in both cases. I've added language-pack-tr to the Build-Depends to make sure the locale is valid during normal build tests (the i problem is more interesting to test than the invalid locale problem, which is probably expected behavior anyway).
[1] http://
Excellent work adding a test case for this! LGTM