Patchwork D7732: convert: refactor authormap into separate function for outside use

login
register
mail settings
Submitter phabricator
Date Dec. 27, 2019, 8:10 p.m.
Message ID <differential-rev-PHID-DREV-gujwbax7aidxrujgm3cd-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44061/
State Superseded
Headers show

Comments

phabricator - Dec. 27, 2019, 8:10 p.m.
joerg.sonnenberger created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/convert/convcmd.py

CHANGE DETAILS




To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 30, 2019, 1:19 p.m.
pulkit added inline comments.

INLINE COMMENTS

> convcmd.py:479
>      def readauthormap(self, authorfile):
> -        afile = open(authorfile, b'rb')
> -        for line in afile:
> -
> -            line = line.strip()
> -            if not line or line.startswith(b'#'):
> -                continue
> -
> -            try:
> -                srcauthor, dstauthor = line.split(b'=', 1)
> -            except ValueError:
> -                msg = _(b'ignoring bad line in author map file %s: %s\n')
> -                self.ui.warn(msg % (authorfile, line.rstrip()))
> -                continue
> -
> -            srcauthor = srcauthor.strip()
> -            dstauthor = dstauthor.strip()
> -            if self.authors.get(srcauthor) in (None, dstauthor):
> -                msg = _(b'mapping author %s to %s\n')
> -                self.ui.debug(msg % (srcauthor, dstauthor))
> -                self.authors[srcauthor] = dstauthor
> -                continue
> -
> -            m = _(b'overriding mapping for author %s, was %s, will be %s\n')
> -            self.ui.status(m % (srcauthor, self.authors[srcauthor], dstauthor))
> -
> -        afile.close()
> +        self.authors = readauthormap(self.ui, authorfile)
>  

This will reassign `self.authors` instead of updating it. `readauthormap` is called twice in the `__init__()`.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7732/new/

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

To: joerg.sonnenberger, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Jan. 8, 2020, 12:07 p.m.
pulkit added inline comments.

INLINE COMMENTS

> convcmd.py:58
>  
> +def readauthormap(ui, authorfile, authors={}):
> +    with open(authorfile, b'rb') as afile:

We disallow usage of default mutable arguments. Hence `test-check-code.t` says hi.

  +  hgext/convert/convcmd.py:58:
  +   > def readauthormap(ui, authorfile, authors={}):
  +   don't use mutable default arguments

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7732/new/

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

To: joerg.sonnenberger, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Jan. 17, 2020, 3:08 p.m.
pulkit added a comment.


  Amending the following in flight to make `test-check-format.t` (or black) happy:
  
    diff --git a/hgext/convert/convcmd.py b/hgext/convert/convcmd.py
    --- a/hgext/convert/convcmd.py
    +++ b/hgext/convert/convcmd.py
    @@ -55,6 +55,7 @@ svn_source = subversion.svn_source
     
     orig_encoding = b'ascii'
     
    +
     def readauthormap(ui, authorfile, authors=None):
         if authors is None:
             authors = {}
    @@ -84,6 +85,7 @@ def readauthormap(ui, authorfile, author
                 ui.status(m % (srcauthor, authors[srcauthor], dstauthor))
         return authors
     
    +
     def recode(s):
         if isinstance(s, pycompat.unicode):
             return s.encode(pycompat.sysstr(orig_encoding), 'replace')

REPOSITORY
  rHG Mercurial

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7732/new/

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

To: joerg.sonnenberger, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel

Patch

diff --git a/hgext/convert/convcmd.py b/hgext/convert/convcmd.py
--- a/hgext/convert/convcmd.py
+++ b/hgext/convert/convcmd.py
@@ -55,6 +55,34 @@ 
 
 orig_encoding = b'ascii'
 
+def readauthormap(ui, authorfile):
+    authors = {}
+
+    with open(authorfile, b'rb') as afile:
+        for line in afile:
+
+            line = line.strip()
+            if not line or line.startswith(b'#'):
+                continue
+
+            try:
+                srcauthor, dstauthor = line.split(b'=', 1)
+            except ValueError:
+                msg = _(b'ignoring bad line in author map file %s: %s\n')
+                ui.warn(msg % (authorfile, line.rstrip()))
+                continue
+
+            srcauthor = srcauthor.strip()
+            dstauthor = dstauthor.strip()
+            if authors.get(srcauthor) in (None, dstauthor):
+                msg = _(b'mapping author %s to %s\n')
+                ui.debug(msg % (srcauthor, dstauthor))
+                authors[srcauthor] = dstauthor
+                continue
+
+            m = _(b'overriding mapping for author %s, was %s, will be %s\n')
+            ui.status(m % (srcauthor, authors[srcauthor], dstauthor))
+    return authors
 
 def recode(s):
     if isinstance(s, pycompat.unicode):
@@ -448,32 +476,7 @@ 
             ofile.close()
 
     def readauthormap(self, authorfile):
-        afile = open(authorfile, b'rb')
-        for line in afile:
-
-            line = line.strip()
-            if not line or line.startswith(b'#'):
-                continue
-
-            try:
-                srcauthor, dstauthor = line.split(b'=', 1)
-            except ValueError:
-                msg = _(b'ignoring bad line in author map file %s: %s\n')
-                self.ui.warn(msg % (authorfile, line.rstrip()))
-                continue
-
-            srcauthor = srcauthor.strip()
-            dstauthor = dstauthor.strip()
-            if self.authors.get(srcauthor) in (None, dstauthor):
-                msg = _(b'mapping author %s to %s\n')
-                self.ui.debug(msg % (srcauthor, dstauthor))
-                self.authors[srcauthor] = dstauthor
-                continue
-
-            m = _(b'overriding mapping for author %s, was %s, will be %s\n')
-            self.ui.status(m % (srcauthor, self.authors[srcauthor], dstauthor))
-
-        afile.close()
+        self.authors = readauthormap(self.ui, authorfile)
 
     def cachecommit(self, rev):
         commit = self.source.getcommit(rev)