D3003: stringutil: improve check for failed mailmap line parsing

2018-03-31 Thread sheehan (Connor Sheehan)
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG0e7550b0964c: stringutil: improve check for failed mailmap 
line parsing (authored by sheehan, committed by ).

CHANGED PRIOR TO COMMIT
  https://phab.mercurial-scm.org/D3003?vs=7479=7484#toc

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D3003?vs=7479=7484

REVISION DETAIL
  https://phab.mercurial-scm.org/D3003

AFFECTED FILES
  mercurial/utils/stringutil.py

CHANGE DETAILS

diff --git a/mercurial/utils/stringutil.py b/mercurial/utils/stringutil.py
--- a/mercurial/utils/stringutil.py
+++ b/mercurial/utils/stringutil.py
@@ -166,6 +166,30 @@
 email = attr.ib()
 name = attr.ib(default=None)
 
+def _ismailmaplineinvalid(names, emails):
+'''Returns True if the parsed names and emails
+in a mailmap entry are invalid.
+
+>>> # No names or emails fails
+>>> names, emails = [], []
+>>> _ismailmaplineinvalid(names, emails)
+True
+>>> # Only one email fails
+>>> emails = [b'em...@email.com']
+>>> _ismailmaplineinvalid(names, emails)
+True
+>>> # One email and one name passes
+>>> names = [b'Test Name']
+>>> _ismailmaplineinvalid(names, emails)
+False
+>>> # No names but two emails passes
+>>> names = []
+>>> emails = [b'pro...@email.com', b'com...@email.com']
+>>> _ismailmaplineinvalid(names, emails)
+False
+'''
+return not emails or not names and len(emails) < 2
+
 def parsemailmap(mailmapcontent):
 """Parses data in the .mailmap format
 
@@ -199,7 +223,7 @@
 
 # Don't bother checking the line if it is a comment or
 # is an improperly formed author field
-if line.lstrip().startswith('#') or any(c not in line for c in '<>@'):
+if line.lstrip().startswith('#'):
 continue
 
 # names, emails hold the parsed emails and names for each line
@@ -230,6 +254,12 @@
 # We have found another word in the committers name
 namebuilder.append(element)
 
+# Check to see if we have parsed the line into a valid form
+# We require at least one email, and either at least one
+# name or a second email
+if _ismailmaplineinvalid(names, emails):
+continue
+
 mailmapkey = mailmapping(
 email=emails[-1],
 name=names[-1] if len(names) == 2 else None,



To: sheehan, #hg-reviewers, yuja
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


D3003: stringutil: improve check for failed mailmap line parsing

2018-03-31 Thread yuja (Yuya Nishihara)
yuja added a comment.


  Queued the series, thanks.

INLINE COMMENTS

> stringutil.py:169
>  
> +def ismailmaplineinvalid(names, emails):
> +'''Returns True if the parsed names and emails

I renamed this to `_ismailmaplineinvalid` since this will never be
publicly used.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3003

To: sheehan, #hg-reviewers, yuja
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


D3003: stringutil: improve check for failed mailmap line parsing

2018-03-31 Thread sheehan (Connor Sheehan)
sheehan created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  The existing check for a bad mailmap file entry fails with inputs
  like b'>@<'. This commit adds a function to check if a sufficient
  amount of information has been parsed from a mailmap file entry.
  
  At minimum, one email must be found (assumed to be the commit email).
  If email is not empty and no names are found, then there must be
  two emails. If there are at least one email and name, the mapping
  is valid.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3003

AFFECTED FILES
  mercurial/utils/stringutil.py

CHANGE DETAILS

diff --git a/mercurial/utils/stringutil.py b/mercurial/utils/stringutil.py
--- a/mercurial/utils/stringutil.py
+++ b/mercurial/utils/stringutil.py
@@ -166,6 +166,30 @@
 email = attr.ib()
 name = attr.ib(default=None)
 
+def ismailmaplineinvalid(names, emails):
+'''Returns True if the parsed names and emails
+in a mailmap entry are invalid.
+
+>>> # No names or emails fails
+>>> names, emails = [], []
+>>> ismailmaplineinvalid(names, emails)
+True
+>>> # Only one email fails
+>>> emails = [b'em...@email.com']
+>>> ismailmaplineinvalid(names, emails)
+True
+>>> # One email and one name passes
+>>> names = [b'Test Name']
+>>> ismailmaplineinvalid(names, emails)
+False
+>>> # No names but two emails passes
+>>> names = []
+>>> emails = [b'pro...@email.com', b'com...@email.com']
+>>> ismailmaplineinvalid(names, emails)
+False
+'''
+return not emails or not names and len(emails) < 2
+
 def parsemailmap(mailmapcontent):
 """Parses data in the .mailmap format
 
@@ -199,7 +223,7 @@
 
 # Don't bother checking the line if it is a comment or
 # is an improperly formed author field
-if line.lstrip().startswith('#') or any(c not in line for c in '<>@'):
+if line.lstrip().startswith('#'):
 continue
 
 # name, email hold the parsed emails and names for each line
@@ -230,6 +254,12 @@
 # We have found another word in the committers name
 namebuilder.append(element)
 
+# Check to see if we have parsed the line into a valid form
+# We require at least one email, and either at least one
+# name or a second email
+if ismailmaplineinvalid(names, emails):
+continue
+
 mailmapkey = mailmapping(
 email=emails[-1],
 name=names[-1] if len(names) == 2 else None,



To: sheehan, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel