Page MenuHomePhabricator

Allow parenthesis in author name
ClosedPublic

Authored by sowedance on Mar 6 2014, 6:26 PM.
Tags
None
Referenced Files
F13083860: D8429.diff
Wed, Apr 24, 10:44 PM
Unknown Object (File)
Sat, Apr 20, 6:34 PM
Unknown Object (File)
Thu, Apr 11, 11:32 PM
Unknown Object (File)
Thu, Apr 11, 8:47 AM
Unknown Object (File)
Thu, Apr 11, 8:47 AM
Unknown Object (File)
Thu, Apr 11, 8:47 AM
Unknown Object (File)
Thu, Apr 11, 8:17 AM
Unknown Object (File)
Thu, Apr 11, 8:01 AM

Details

Reviewers
wez
epriestley
lifeihuang
JoelB
Group Reviewers
Blessed Reviewers
Commits
Restricted Diffusion Commit
rP81d9935efe92: Allow parenthesis in author name
Summary

We have a dozen users who has (...) in their 'real name', like 'Jimmy (He) Zhang', and it's causing the diffusion file browser problems when blame is enabled. The parser does not expect those parenthesis and the lines of code will be empty if they were last touched by a user like that.

Test Plan

Try it

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I think this will incorrectly match too much for inputs like this (where the line of source code looks like a blame line, as some of the lines in this file do):

abcd (first (middle) last 2010-12-03 1) code(); "(string literal 9999-99-99 2)"; more_code();

Specifically, I think the user's name will be parsed as:

first (middle) last 2010-12-03 1) code(); "string literal

You can probably fix this by using a ? to make the match ungreedy (that is, (.*?) instead of (.*)). We could also make this a public static method and write a couple of unit tests for it to be more certain.

epriestley edited edge metadata.

(As above.)

This revision now requires changes to proceed.Mar 6 2014, 6:30 PM

Ah yes. Not likely to have this kind of lines but still need to be considered. I'll change it and add some tests.

sowedance updated this revision to Unknown Object (????).Mar 6 2014, 7:22 PM

Make the match ungreedy and added a test case to verify it.

This revision is now accepted and ready to land.Mar 6 2014, 7:27 PM