Do not use dunder prefix for attribute names

Instance attribute names using a dunder ('__') prefix can make debugging
more difficult due to how Python mangles such names at runtime. And as used
in stgit, these dunder prefixed attributes do not fit the nominal use case
for dunder prefixes to help with name clashes between super and sub
classes:

https://docs.python.org/3/reference/lexical_analysis.html#reserved-classes-of-identifiers

Some instance attributes with dunder prefixes are renamed to use a single
'_'. This still signals to readers that the attribute is private, but
allows the variable to accessed with less trouble in a debug context.

Some other instance attributes with dunder prefixes that serve to back
properties are replaced along with the property with regular (non-property)
attributes.

Signed-off-by: Peter Grayson <[email protected]>

Author: jpgrayson

stgit/commands/branch.py

@@ -181,7 +181,7 @@
 
 
 def __is_current_branch(branch_name):
-    return crt_series.get_name() == branch_name
+    return crt_series.name == branch_name
 
 
 def __print_branch(branch_name, length):
@@ -295,7 +295,7 @@ def func(parser, options, args):
     elif options.clone:
 
         if len(args) == 0:
-            clone = crt_series.get_name() + time.strftime('-%C%y%m%d-%H%M%S')
+            clone = crt_series.name + time.strftime('-%C%y%m%d-%H%M%S')
         elif len(args) == 1:
             clone = args[0]
         else:
@@ -309,7 +309,7 @@ def func(parser, options, args):
         crt_series.clone(clone)
         out.done()
 
-        log.copy_log(log.default_repo(), crt_series.get_name(), clone,
+        log.copy_log(log.default_repo(), crt_series.name, clone,
                      'branch --clone')
         return
 
@@ -324,7 +324,7 @@ def func(parser, options, args):
     elif options.cleanup:
 
         if not args:
-            name = crt_series.get_name()
+            name = crt_series.name
         elif len(args) == 1:
             name = args[0]
         else:
@@ -356,7 +356,7 @@ def func(parser, options, args):
     elif options.protect:
 
         if len(args) == 0:
-            branch_name = crt_series.get_name()
+            branch_name = crt_series.name
         elif len(args) == 1:
             branch_name = args[0]
         else:
@@ -390,7 +390,7 @@ def func(parser, options, args):
     elif options.unprotect:
 
         if len(args) == 0:
-            branch_name = crt_series.get_name()
+            branch_name = crt_series.name
         elif len(args) == 1:
             branch_name = args[0]
         else:
@@ -410,7 +410,7 @@ def func(parser, options, args):
     elif options.description is not None:
 
         if len(args) == 0:
-            branch_name = crt_series.get_name()
+            branch_name = crt_series.name
         elif len(args) == 1:
             branch_name = args[0]
         else:
@@ -445,4 +445,4 @@ def func(parser, options, args):
     if len(args) != 0:
         parser.error('incorrect number of arguments')
 
-    out.stdout(crt_series.get_name())
+    out.stdout(crt_series.name)

stgit/commands/common.py

@@ -77,7 +77,7 @@ def git_id(crt_series, rev):
     # TODO: remove this function once all the occurrences were converted
     # to git_commit()
     repository = StackRepository.default()
-    return git_commit(rev, repository, crt_series.get_name()).sha1
+    return git_commit(rev, repository, crt_series.name).sha1
 
 
 def get_public_ref(branch_name):

stgit/commands/repair.py

@@ -91,13 +91,13 @@ def __init__(self, id):
         self.parents = set()
         self.children = set()
         self.patch = None
-        self.__commit = None
+        self._commit = None
 
-    def __get_commit(self):
-        if not self.__commit:
-            self.__commit = git.get_commit(self.id)
-        return self.__commit
-    commit = property(__get_commit)
+    @property
+    def commit(self):
+        if not self._commit:
+            self._commit = git.get_commit(self.id)
+        return self._commit
 
     def __str__(self):
         if self.patch:
@@ -147,7 +147,7 @@ def func(parser, options, args):
 
     # Find commits that aren't patches, and applied patches.
     head = git.get_commit(git.get_head()).get_id_hash()
-    commits, patches = read_commit_dag(crt_series.get_name())
+    commits, patches = read_commit_dag(crt_series.name)
     c = commits[head]
     patchify = []        # commits to definitely patchify
     maybe_patchify = []  # commits to patchify if we find a patch below them

stgit/commands/sync.py

@@ -100,7 +100,7 @@ def func(parser, options, args):
     """
     if options.ref_branch:
         remote_series = stack.Series(options.ref_branch)
-        if options.ref_branch == crt_series.get_name():
+        if options.ref_branch == crt_series.name:
             raise CmdException('Cannot synchronise with the current branch')
         remote_patches = remote_series.get_applied()
 

stgit/config.py

@@ -55,14 +55,13 @@ class GitConfigException(StgException):
 
 class GitConfig(object):
 
-    __cache = None
+    _cache = None
 
     def load(self):
-        """Load the whole configuration in __cache unless it has been
-        done already."""
-        if self.__cache is not None:
+        """Load the configuration in _cache unless it has been done already."""
+        if self._cache is not None:
             return
-        self.__cache = DEFAULTS.copy()
+        self._cache = DEFAULTS.copy()
         lines = Run('git', 'config', '--null', '--list'
                     ).discard_exitcode().output_lines('\0')
         for line in lines:
@@ -71,19 +70,19 @@ def load(self):
             except ValueError:
                 key = line
                 value = None
-            self.__cache.setdefault(key, []).append(value)
+            self._cache.setdefault(key, []).append(value)
 
     def get(self, name):
         self.load()
         try:
-            return self.__cache[name][-1]
+            return self._cache[name][-1]
         except KeyError:
             return None
 
     def getall(self, name):
         self.load()
         try:
-            return self.__cache[name]
+            return self._cache[name]
         except KeyError:
             return []
 
@@ -105,7 +104,7 @@ def getbool(self, name):
         # reported as "true".
         self.load()
         try:
-            value = self.__cache[name][-1]
+            value = self._cache[name][-1]
         except KeyError:
             return None
         if value is None:
@@ -123,30 +122,30 @@ def getbool(self, name):
 
     def getstartswith(self, name):
         self.load()
-        return ((n, v[-1]) for (n, v) in self.__cache.items()
+        return ((n, v[-1]) for (n, v) in self._cache.items()
                 if n.startswith(name))
 
     def rename_section(self, from_name, to_name):
         """Rename a section in the config file. Silently do nothing if
         the section doesn't exist."""
         Run('git', 'config', '--rename-section', from_name, to_name
             ).returns([0, 1, 128]).run()
-        self.__cache.clear()
+        self._cache.clear()
 
     def remove_section(self, name):
         """Remove a section in the config file. Silently do nothing if
         the section doesn't exist."""
         Run('git', 'config', '--remove-section', name
             ).returns([0, 1, 128]).discard_stderr().discard_output()
-        self.__cache.clear()
+        self._cache.clear()
 
     def set(self, name, value):
         Run('git', 'config', name, value).run()
-        self.__cache[name] = value
+        self._cache[name] = value
 
     def unset(self, name):
         Run('git', 'config', '--unset', name).run()
-        self.__cache[name] = [None]
+        self._cache[name] = [None]
 
     def sections_matching(self, regexp):
         """Takes a regexp with a single group, matches it against all

stgit/git.py

@@ -107,7 +107,7 @@ class Commit(object):
     """
 
     def __init__(self, id_hash):
-        self.__id_hash = id_hash
+        self._id_hash = id_hash
 
         lines = GRun('cat-file', 'commit', id_hash).output_lines()
         for i in range(len(lines)):
@@ -116,20 +116,20 @@ def __init__(self, id_hash):
                 break  # we've seen all the header fields
             key, val = line.split(' ', 1)
             if key == 'tree':
-                self.__tree = val
+                self._tree = val
             elif key == 'author':
-                self.__author = val
+                self._author = val
             elif key == 'committer':
-                self.__committer = val
+                self._committer = val
             else:
                 pass  # ignore other headers
-        self.__log = '\n'.join(lines[i + 1:])
+        self._log = '\n'.join(lines[i + 1:])
 
     def get_id_hash(self):
-        return self.__id_hash
+        return self._id_hash
 
     def get_tree(self):
-        return self.__tree
+        return self._tree
 
     def get_parent(self):
         parents = self.get_parents()
@@ -139,17 +139,17 @@ def get_parent(self):
             return None
 
     def get_parents(self):
-        return GRun('rev-list', '--parents', '--max-count=1', self.__id_hash
+        return GRun('rev-list', '--parents', '--max-count=1', self._id_hash
                     ).output_one_line().split()[1:]
 
     def get_author(self):
-        return self.__author
+        return self._author
 
     def get_committer(self):
-        return self.__committer
+        return self._committer
 
     def get_log(self):
-        return self.__log
+        return self._log
 
     def __str__(self):
         return self.get_id_hash()

stgit/lib/git/branch.py

@@ -18,52 +18,48 @@ class Branch(object):
     """Represents a Git branch."""
 
     def __init__(self, repository, name):
-        self.__repository = repository
-        self.__name = name
+        self.repository = repository
+        self.name = name
         try:
             self.head
         except KeyError:
             raise BranchException('%s: no such branch' % name)
 
     @property
-    def name(self):
-        return self.__name
+    def _ref(self):
+        return 'refs/heads/%s' % self.name
 
     @property
-    def repository(self):
-        return self.__repository
-
-    def __ref(self):
-        return 'refs/heads/%s' % self.__name
+    def _remote_key(self):
+        return 'branch.%s.remote' % self.name
 
     @property
     def head(self):
-        return self.__repository.refs.get(self.__ref())
+        return self.repository.refs.get(self._ref)
 
     def set_head(self, commit, msg):
-        self.__repository.refs.set(self.__ref(), commit, msg)
+        self.repository.refs.set(self._ref, commit, msg)
 
     @property
     def parent_remote(self):
-        remote_key = 'branch.%s.remote' % self.__name
-        remote = config.get(remote_key)
+        remote = config.get(self._remote_key)
         if remote is None:
             raise BranchException(
                 '%s: no parent remote; consider configuring "%s"' % (
-                    self.__name, remote_key
+                    self.name, self._remote_key
                 )
             )
         else:
             return remote
 
     def set_parent_remote(self, name):
-        config.set('branch.%s.remote' % self.__name, name)
+        config.set('branch.%s.remote' % self.name, name)
 
     def set_parent_branch(self, name):
-        if config.get('branch.%s.remote' % self.__name):
+        if config.get(self._remote_key):
             # Never set merge if remote is not set to avoid
             # possibly-erroneous lookups into 'origin'
-            config.set('branch.%s.merge' % self.__name, name)
+            config.set('branch.%s.merge' % self.name, name)
 
     @classmethod
     def create(cls, repository, name, create_at=None):