Merge lp://staging/~robert-ancell/indicator-session/blank-username into lp://staging/indicator-session/14.04

Proposed by Robert Ancell
Status: Merged
Approved by: Ted Gould
Approved revision: 425
Merged at revision: 425
Proposed branch: lp://staging/~robert-ancell/indicator-session/blank-username
Merge into: lp://staging/indicator-session/14.04
Diff against target: 88 lines (+46/-3)
2 files modified
src/service.c (+16/-3)
tests/test-service.cc (+30/-0)
To merge this branch: bzr merge lp://staging/~robert-ancell/indicator-session/blank-username
Reviewer Review Type Date Requested Status
Ted Gould (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+199604@code.staging.launchpad.net

Commit message

Use username as label if the real name is empty or whitespace

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ted Gould (ted) wrote :

What about using g_unichar_isspace() since we expect the real name to be
a UTF8 string?

Revision history for this message
Robert Ancell (robert-ancell) wrote :

I had a long look at what it takes to parse it as UTF-8, I'm not sure if it's worth the effort.
- We really can't be sure it is valid UTF-8 so we have to validate it at the top of the function
- We have to handle invalid UTF8 cases which makes the code more complex
- This only helps if there are only some obscure non-printable UTF8 characters used - it's completely valid to check for the ASCII characters as all the UTF8 will be ignored

So I'd say "don't bother", what do you think?

Revision history for this message
Ted Gould (ted) wrote :

I guess my concern was that the UTF-8 code would be a multibyte code, where one of those bytes is a space. So we'd improperly detect a space when really it was a valid visible UTF-8 character. But since what we're detecting is all spaces, that would require both bytes to be valid ASCII spaces, and the name to be a single multi-byte character in length. Which seems pretty rare.

review: Approve

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