Merge lp://qastaging/~mterry/unity-scopes-api/turkish into lp://qastaging/unity-scopes-api

Proposed by Michael Terry
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
Reviewer Review Type Date Requested Status
Marcus Tomlinson (community) Approve
Review via email: mp+307967@code.qastaging.launchpad.net

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-specified locale is not set up correctly, an exception will be thrown. Fair enough I guess, but still a harsh possible side effect of just trying to lowercase some ASCII characters.

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://haacked.com/archive/2012/07/05/turkish-i-problem-and-why-you-should-care.aspx/

To post a comment you must log in.
Revision history for this message
Marcus Tomlinson (marcustomlinson) wrote :

Excellent work adding a test case for this! LGTM

review: Approve
372. By Michael Terry

Merge devel

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: