Abstract a PatchSet object out of Series.
[stgit] / stgit / stack.py
index 3632aa1..0ecac2c 100644 (file)
@@ -23,6 +23,7 @@ import sys, os, re
 from stgit.utils import *
 from stgit import git, basedir, templates
 from stgit.config import config
+from shutil import copyfile
 
 
 # stack exception class
@@ -91,19 +92,7 @@ def edit_file(series, line, comment, show_patch = True):
     print >> f, __comment_prefix, 'vi: set textwidth=75 filetype=diff nobackup:'
     f.close()
 
-    # the editor
-    editor = config.get('stgit.editor')
-    if editor:
-        pass
-    elif 'EDITOR' in os.environ:
-        editor = os.environ['EDITOR']
-    else:
-        editor = 'vi'
-    editor += ' %s' % fname
-
-    print 'Invoking the editor: "%s"...' % editor,
-    sys.stdout.flush()
-    print 'done (exit code: %d)' % os.system(editor)
+    call_editor(fname)
 
     f = file(fname, 'r+')
 
@@ -284,8 +273,137 @@ class Patch(StgitObject):
         self._set_field('log', value)
         self.__update_log_ref(value)
 
+# The current StGIT metadata format version.
+FORMAT_VERSION = 2
+
+def format_version_key(branch):
+    return 'branch.%s.stgitformatversion' % branch
+
+def update_to_current_format_version(branch, git_dir):
+    """Update a potentially older StGIT directory structure to the
+    latest version. Note: This function should depend as little as
+    possible on external functions that may change during a format
+    version bump, since it must remain able to process older formats."""
+
+    branch_dir = os.path.join(git_dir, 'patches', branch)
+    def get_format_version():
+        """Return the integer format version number, or None if the
+        branch doesn't have any StGIT metadata at all, of any version."""
+        fv = config.get(format_version_key(branch))
+        if fv:
+            # Great, there's an explicitly recorded format version
+            # number, which means that the branch is initialized and
+            # of that exact version.
+            return int(fv)
+        elif os.path.isdir(os.path.join(branch_dir, 'patches')):
+            # There's a .git/patches/<branch>/patches dirctory, which
+            # means this is an initialized version 1 branch.
+            return 1
+        elif os.path.isdir(branch_dir):
+            # There's a .git/patches/<branch> directory, which means
+            # this is an initialized version 0 branch.
+            return 0
+        else:
+            # The branch doesn't seem to be initialized at all.
+            return None
+    def set_format_version(v):
+        out.info('Upgraded branch %s to format version %d' % (branch, v))
+        config.set(format_version_key(branch), '%d' % v)
+    def mkdir(d):
+        if not os.path.isdir(d):
+            os.makedirs(d)
+    def rm(f):
+        if os.path.exists(f):
+            os.remove(f)
+
+    # Update 0 -> 1.
+    if get_format_version() == 0:
+        mkdir(os.path.join(branch_dir, 'trash'))
+        patch_dir = os.path.join(branch_dir, 'patches')
+        mkdir(patch_dir)
+        refs_dir = os.path.join(git_dir, 'refs', 'patches', branch)
+        mkdir(refs_dir)
+        for patch in (file(os.path.join(branch_dir, 'unapplied')).readlines()
+                      + file(os.path.join(branch_dir, 'applied')).readlines()):
+            patch = patch.strip()
+            os.rename(os.path.join(branch_dir, patch),
+                      os.path.join(patch_dir, patch))
+            Patch(patch, patch_dir, refs_dir).update_top_ref()
+        set_format_version(1)
+
+    # Update 1 -> 2.
+    if get_format_version() == 1:
+        desc_file = os.path.join(branch_dir, 'description')
+        if os.path.isfile(desc_file):
+            desc = read_string(desc_file)
+            if desc:
+                config.set('branch.%s.description' % branch, desc)
+            rm(desc_file)
+        rm(os.path.join(branch_dir, 'current'))
+        rm(os.path.join(git_dir, 'refs', 'bases', branch))
+        set_format_version(2)
+
+    # Make sure we're at the latest version.
+    if not get_format_version() in [None, FORMAT_VERSION]:
+        raise StackException('Branch %s is at format version %d, expected %d'
+                             % (branch, get_format_version(), FORMAT_VERSION))
+
+class PatchSet(StgitObject):
+    def get_name(self):
+        return self.__name
+    def set_name(self, name):
+        self.__name = name
+
+    def get_head(self):
+        """Return the head of the branch
+        """
+        crt = self.get_current_patch()
+        if crt:
+            return crt.get_top()
+        else:
+            return self.get_base()
+
+    def get_protected(self):
+        return os.path.isfile(os.path.join(self._dir(), 'protected'))
+
+    def protect(self):
+        protect_file = os.path.join(self._dir(), 'protected')
+        if not os.path.isfile(protect_file):
+            create_empty_file(protect_file)
+
+    def unprotect(self):
+        protect_file = os.path.join(self._dir(), 'protected')
+        if os.path.isfile(protect_file):
+            os.remove(protect_file)
+
+    def __branch_descr(self):
+        return 'branch.%s.description' % self.get_name()
+
+    def get_description(self):
+        return config.get(self.__branch_descr()) or ''
+
+    def set_description(self, line):
+        if line:
+            config.set(self.__branch_descr(), line)
+        else:
+            config.unset(self.__branch_descr())
+
+    def head_top_equal(self):
+        """Return true if the head and the top are the same
+        """
+        crt = self.get_current_patch()
+        if not crt:
+            # we don't care, no patches applied
+            return True
+        return git.get_head() == crt.get_top()
+
+    def is_initialised(self):
+        """Checks if series is already initialised
+        """
+        return bool(config.get(format_version_key(self.get_name())))
+
 
-class Series(StgitObject):
+class Series(PatchSet):
     """Class including the operations on series
     """
     def __init__(self, name = None):
@@ -293,40 +411,31 @@ class Series(StgitObject):
         """
         try:
             if name:
-                self.__name = name
+                self.set_name (name)
             else:
-                self.__name = git.get_head_file()
+                self.set_name (git.get_head_file())
             self.__base_dir = basedir.get()
         except git.GitException, ex:
             raise StackException, 'GIT tree not initialised: %s' % ex
 
-        self._set_dir(os.path.join(self.__base_dir, 'patches', self.__name))
+        self._set_dir(os.path.join(self.__base_dir, 'patches', self.get_name()))
+
+        # Update the branch to the latest format version if it is
+        # initialized, but don't touch it if it isn't.
+        update_to_current_format_version(self.get_name(), self.__base_dir)
+
         self.__refs_dir = os.path.join(self.__base_dir, 'refs', 'patches',
-                                       self.__name)
-        self.__base_file = os.path.join(self.__base_dir, 'refs', 'bases',
-                                        self.__name)
+                                       self.get_name())
 
         self.__applied_file = os.path.join(self._dir(), 'applied')
         self.__unapplied_file = os.path.join(self._dir(), 'unapplied')
         self.__hidden_file = os.path.join(self._dir(), 'hidden')
-        self.__current_file = os.path.join(self._dir(), 'current')
-        self.__descr_file = os.path.join(self._dir(), 'description')
 
         # where this series keeps its patches
         self.__patch_dir = os.path.join(self._dir(), 'patches')
-        if not os.path.isdir(self.__patch_dir):
-            self.__patch_dir = self._dir()
-
-        # if no __refs_dir, create and populate it (upgrade old repositories)
-        if self.is_initialised() and not os.path.isdir(self.__refs_dir):
-            os.makedirs(self.__refs_dir)
-            for patch in self.get_applied() + self.get_unapplied():
-                self.get_patch(patch).update_top_ref()
 
         # trash directory
         self.__trash_dir = os.path.join(self._dir(), 'trash')
-        if self.is_initialised() and not os.path.isdir(self.__trash_dir):
-            os.makedirs(self.__trash_dir)
 
     def __patch_name_valid(self, name):
         """Raise an exception if the patch name is not valid.
@@ -334,16 +443,6 @@ class Series(StgitObject):
         if not name or re.search('[^\w.-]', name):
             raise StackException, 'Invalid patch name: "%s"' % name
 
-    def get_branch(self):
-        """Return the branch name for the Series object
-        """
-        return self.__name
-
-    def __set_current(self, name):
-        """Sets the topmost patch
-        """
-        self._set_field('current', name)
-
     def get_patch(self, name):
         """Return a Patch object for the given name
         """
@@ -360,104 +459,85 @@ class Series(StgitObject):
     def get_current(self):
         """Return the name of the topmost patch, or None if there is
         no such patch."""
-        name = self._get_field('current')
-        if name == '':
+        try:
+            applied = self.get_applied()
+        except StackException:
+            # No "applied" file: branch is not initialized.
+            return None
+        try:
+            return applied[-1]
+        except IndexError:
+            # No patches applied.
             return None
-        else:
-            return name
 
     def get_applied(self):
         if not os.path.isfile(self.__applied_file):
-            raise StackException, 'Branch "%s" not initialised' % self.__name
-        f = file(self.__applied_file)
-        names = [line.strip() for line in f.readlines()]
-        f.close()
-        return names
+            raise StackException, 'Branch "%s" not initialised' % self.get_name()
+        return read_strings(self.__applied_file)
 
     def get_unapplied(self):
         if not os.path.isfile(self.__unapplied_file):
-            raise StackException, 'Branch "%s" not initialised' % self.__name
-        f = file(self.__unapplied_file)
-        names = [line.strip() for line in f.readlines()]
-        f.close()
-        return names
+            raise StackException, 'Branch "%s" not initialised' % self.get_name()
+        return read_strings(self.__unapplied_file)
 
     def get_hidden(self):
         if not os.path.isfile(self.__hidden_file):
             return []
-        f = file(self.__hidden_file)
-        names = [line.strip() for line in f.readlines()]
-        f.close()
-        return names
-
-    def get_base_file(self):
-        self.__begin_stack_check()
-        return self.__base_file
-
-    def get_protected(self):
-        return os.path.isfile(os.path.join(self._dir(), 'protected'))
-
-    def protect(self):
-        protect_file = os.path.join(self._dir(), 'protected')
-        if not os.path.isfile(protect_file):
-            create_empty_file(protect_file)
+        return read_strings(self.__hidden_file)
 
-    def unprotect(self):
-        protect_file = os.path.join(self._dir(), 'protected')
-        if os.path.isfile(protect_file):
-            os.remove(protect_file)
-
-    def get_description(self):
-        return self._get_field('description') or ''
-
-    def set_description(self, line):
-        self._set_field('description', line)
+    def get_base(self):
+        # Return the parent of the bottommost patch, if there is one.
+        if os.path.isfile(self.__applied_file):
+            bottommost = file(self.__applied_file).readline().strip()
+            if bottommost:
+                return self.get_patch(bottommost).get_bottom()
+        # No bottommost patch, so just return HEAD
+        return git.get_head()
 
     def get_parent_remote(self):
-        value = config.get('branch.%s.remote' % self.__name)
+        value = config.get('branch.%s.remote' % self.get_name())
         if value:
             return value
         elif 'origin' in git.remotes_list():
-            print 'Notice: no parent remote declared for stack "%s", ' \
-                  'defaulting to "origin". Consider setting "branch.%s.remote" ' \
-                  'and "branch.%s.merge" with "git repo-config".' \
-                  % (self.__name, self.__name, self.__name)
+            out.note(('No parent remote declared for stack "%s",'
+                      ' defaulting to "origin".' % self.get_name()),
+                     ('Consider setting "branch.%s.remote" and'
+                      ' "branch.%s.merge" with "git repo-config".'
+                      % (self.get_name(), self.get_name())))
             return 'origin'
         else:
-            raise StackException, 'Cannot find a parent remote for "%s"' % self.__name
+            raise StackException, 'Cannot find a parent remote for "%s"' % self.get_name()
 
     def __set_parent_remote(self, remote):
-        value = config.set('branch.%s.remote' % self.__name, remote)
+        value = config.set('branch.%s.remote' % self.get_name(), remote)
 
     def get_parent_branch(self):
-        value = config.get('branch.%s.stgit.parentbranch' % self.__name)
+        value = config.get('branch.%s.stgit.parentbranch' % self.get_name())
         if value:
             return value
         elif git.rev_parse('heads/origin'):
-            print 'Notice: no parent branch declared for stack "%s", ' \
-                  'defaulting to "heads/origin". Consider setting ' \
-                  '"branch.%s.stgit.parentbranch" with "git repo-config".' \
-                  % (self.__name, self.__name)
+            out.note(('No parent branch declared for stack "%s",'
+                      ' defaulting to "heads/origin".' % self.get_name()),
+                     ('Consider setting "branch.%s.stgit.parentbranch"'
+                      ' with "git repo-config".' % self.get_name()))
             return 'heads/origin'
         else:
-            raise StackException, 'Cannot find a parent branch for "%s"' % self.__name
+            raise StackException, 'Cannot find a parent branch for "%s"' % self.get_name()
 
     def __set_parent_branch(self, name):
-        if config.get('branch.%s.remote' % self.__name):
+        if config.get('branch.%s.remote' % self.get_name()):
             # 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.stgit.parentbranch' % self.__name, name)
+            config.set('branch.%s.merge' % self.get_name(), name)
+        config.set('branch.%s.stgit.parentbranch' % self.get_name(), name)
 
     def set_parent(self, remote, localbranch):
-        # policy: record local branches as remote='.'
-        recordremote = remote or '.'
         if localbranch:
-            self.__set_parent_remote(recordremote)
+            self.__set_parent_remote(remote)
             self.__set_parent_branch(localbranch)
         # We'll enforce this later
 #         else:
-#             raise StackException, 'Parent branch (%s) should be specified for %s' % localbranch, self.__name
+#             raise StackException, 'Parent branch (%s) should be specified for %s' % localbranch, self.get_name()
 
     def __patch_is_current(self, patch):
         return patch.get_name() == self.get_current()
@@ -480,101 +560,30 @@ class Series(StgitObject):
     def patch_exists(self, name):
         """Return true if there is a patch with the given name, false
         otherwise."""
-        return self.patch_applied(name) or self.patch_unapplied(name)
-
-    def __begin_stack_check(self):
-        """Save the current HEAD into .git/refs/heads/base if the stack
-        is empty
-        """
-        if len(self.get_applied()) == 0:
-            head = git.get_head()
-            write_string(self.__base_file, head)
-
-    def __end_stack_check(self):
-        """Remove .git/refs/heads/base if the stack is empty.
-        This warning should never happen
-        """
-        if len(self.get_applied()) == 0 \
-           and read_string(self.__base_file) != git.get_head():
-            print 'Warning: stack empty but the HEAD and base are different'
-
-    def head_top_equal(self):
-        """Return true if the head and the top are the same
-        """
-        crt = self.get_current_patch()
-        if not crt:
-            # we don't care, no patches applied
-            return True
-        return git.get_head() == crt.get_top()
-
-    def is_initialised(self):
-        """Checks if series is already initialised
-        """
-        return os.path.isdir(self.__patch_dir)
+        return self.patch_applied(name) or self.patch_unapplied(name) \
+               or self.patch_hidden(name)
 
     def init(self, create_at=False, parent_remote=None, parent_branch=None):
         """Initialises the stgit series
         """
-        bases_dir = os.path.join(self.__base_dir, 'refs', 'bases')
-
-        if os.path.exists(self.__patch_dir):
-            raise StackException, self.__patch_dir + ' already exists'
-        if os.path.exists(self.__refs_dir):
-            raise StackException, self.__refs_dir + ' already exists'
-        if os.path.exists(self.__base_file):
-            raise StackException, self.__base_file + ' already exists'
+        if self.is_initialised():
+            raise StackException, '%s already initialized' % self.get_name()
+        for d in [self._dir(), self.__refs_dir]:
+            if os.path.exists(d):
+                raise StackException, '%s already exists' % d
 
         if (create_at!=False):
-            git.create_branch(self.__name, create_at)
+            git.create_branch(self.get_name(), create_at)
 
         os.makedirs(self.__patch_dir)
 
         self.set_parent(parent_remote, parent_branch)
-        
-        create_dirs(bases_dir)
 
         self.create_empty_field('applied')
         self.create_empty_field('unapplied')
-        self.create_empty_field('description')
-        os.makedirs(os.path.join(self._dir(), 'patches'))
         os.makedirs(self.__refs_dir)
-        self.__begin_stack_check()
-
-    def convert(self):
-        """Either convert to use a separate patch directory, or
-        unconvert to place the patches in the same directory with
-        series control files
-        """
-        if self.__patch_dir == self._dir():
-            print 'Converting old-style to new-style...',
-            sys.stdout.flush()
-
-            self.__patch_dir = os.path.join(self._dir(), 'patches')
-            os.makedirs(self.__patch_dir)
-
-            for p in self.get_applied() + self.get_unapplied():
-                src = os.path.join(self._dir(), p)
-                dest = os.path.join(self.__patch_dir, p)
-                os.rename(src, dest)
 
-            print 'done'
-
-        else:
-            print 'Converting new-style to old-style...',
-            sys.stdout.flush()
-
-            for p in self.get_applied() + self.get_unapplied():
-                src = os.path.join(self.__patch_dir, p)
-                dest = os.path.join(self._dir(), p)
-                os.rename(src, dest)
-
-            if not os.listdir(self.__patch_dir):
-                os.rmdir(self.__patch_dir)
-                print 'done'
-            else:
-                print 'Patch directory %s is not empty.' % self.__name
-
-            self.__patch_dir = self._dir()
+        config.set(format_version_key(self.get_name()), str(FORMAT_VERSION))
 
     def rename(self, to_name):
         """Renames a series
@@ -582,24 +591,19 @@ class Series(StgitObject):
         to_stack = Series(to_name)
 
         if to_stack.is_initialised():
-            raise StackException, '"%s" already exists' % to_stack.get_branch()
-        if os.path.exists(to_stack.__base_file):
-            os.remove(to_stack.__base_file)
+            raise StackException, '"%s" already exists' % to_stack.get_name()
 
-        git.rename_branch(self.__name, to_name)
+        git.rename_branch(self.get_name(), to_name)
 
         if os.path.isdir(self._dir()):
             rename(os.path.join(self.__base_dir, 'patches'),
-                   self.__name, to_stack.__name)
-        if os.path.exists(self.__base_file):
-            rename(os.path.join(self.__base_dir, 'refs', 'bases'),
-                   self.__name, to_stack.__name)
+                   self.get_name(), to_stack.get_name())
         if os.path.exists(self.__refs_dir):
             rename(os.path.join(self.__base_dir, 'refs', 'patches'),
-                   self.__name, to_stack.__name)
+                   self.get_name(), to_stack.get_name())
 
         # Rename the config section
-        config.rename_section("branch.%s" % self.__name,
+        config.rename_section("branch.%s" % self.get_name(),
                               "branch.%s" % to_name)
 
         self.__init__(to_name)
@@ -609,14 +613,14 @@ class Series(StgitObject):
         """
         try:
             # allow cloning of branches not under StGIT control
-            base = read_string(self.get_base_file())
+            base = self.get_base()
         except:
             base = git.get_head()
         Series(target_series).init(create_at = base)
         new_series = Series(target_series)
 
         # generate an artificial description file
-        new_series.set_description('clone of "%s"' % self.__name)
+        new_series.set_description('clone of "%s"' % self.get_name())
 
         # clone self's entire series as unapplied patches
         try:
@@ -629,26 +633,35 @@ class Series(StgitObject):
             patches = applied = unapplied = []
         for p in patches:
             patch = self.get_patch(p)
-            new_series.new_patch(p, message = patch.get_description(),
-                                 can_edit = False, unapplied = True,
-                                 bottom = patch.get_bottom(),
-                                 top = patch.get_top(),
-                                 author_name = patch.get_authname(),
-                                 author_email = patch.get_authemail(),
-                                 author_date = patch.get_authdate())
+            newpatch = new_series.new_patch(p, message = patch.get_description(),
+                                            can_edit = False, unapplied = True,
+                                            bottom = patch.get_bottom(),
+                                            top = patch.get_top(),
+                                            author_name = patch.get_authname(),
+                                            author_email = patch.get_authemail(),
+                                            author_date = patch.get_authdate())
+            if patch.get_log():
+                out.info('Setting log to %s' %  patch.get_log())
+                newpatch.set_log(patch.get_log())
+            else:
+                out.info('No log for %s' % p)
 
         # fast forward the cloned series to self's top
         new_series.forward_patches(applied)
 
-        # Clone remote and merge settings
-        value = config.get('branch.%s.remote' % self.__name)
+        # Clone parent informations
+        value = config.get('branch.%s.remote' % self.get_name())
         if value:
             config.set('branch.%s.remote' % target_series, value)
 
-        value = config.get('branch.%s.merge' % self.__name)
+        value = config.get('branch.%s.merge' % self.get_name())
         if value:
             config.set('branch.%s.merge' % target_series, value)
 
+        value = config.get('branch.%s.stgit.parentbranch' % self.get_name())
+        if value:
+            config.set('branch.%s.stgit.parentbranch' % target_series, value)
+
     def delete(self, force = False):
         """Deletes an stgit series
         """
@@ -660,10 +673,11 @@ class Series(StgitObject):
             for p in patches:
                 Patch(p, self.__patch_dir, self.__refs_dir).delete()
 
-            # remove the trash directory
-            for fname in os.listdir(self.__trash_dir):
-                os.remove(fname)
-            os.rmdir(self.__trash_dir)
+            # remove the trash directory if any
+            if os.path.exists(self.__trash_dir):
+                for fname in os.listdir(self.__trash_dir):
+                    os.remove(os.path.join(self.__trash_dir, fname))
+                os.rmdir(self.__trash_dir)
 
             # FIXME: find a way to get rid of those manual removals
             # (move functionality to StgitObject ?)
@@ -673,28 +687,32 @@ class Series(StgitObject):
                 os.remove(self.__unapplied_file)
             if os.path.exists(self.__hidden_file):
                 os.remove(self.__hidden_file)
-            if os.path.exists(self.__current_file):
-                os.remove(self.__current_file)
-            if os.path.exists(self.__descr_file):
-                os.remove(self.__descr_file)
+            if os.path.exists(self._dir()+'/orig-base'):
+                os.remove(self._dir()+'/orig-base')
+
             if not os.listdir(self.__patch_dir):
                 os.rmdir(self.__patch_dir)
             else:
-                print 'Patch directory %s is not empty.' % self.__name
-            if not os.listdir(self._dir()):
-                remove_dirs(os.path.join(self.__base_dir, 'patches'),
-                            self.__name)
-            else:
-                print 'Series directory %s is not empty.' % self.__name
-            if not os.listdir(self.__refs_dir):
-                remove_dirs(os.path.join(self.__base_dir, 'refs', 'patches'),
-                            self.__name)
-            else:
-                print 'Refs directory %s is not empty.' % self.__refs_dir
-
-        if os.path.exists(self.__base_file):
-            remove_file_and_dirs(
-                os.path.join(self.__base_dir, 'refs', 'bases'), self.__name)
+                out.warn('Patch directory %s is not empty' % self.__patch_dir)
+
+            try:
+                os.removedirs(self._dir())
+            except OSError:
+                raise StackException('Series directory %s is not empty'
+                                     % self._dir())
+
+            try:
+                os.removedirs(self.__refs_dir)
+            except OSError:
+                out.warn('Refs directory %s is not empty' % self.__refs_dir)
+
+        # Cleanup parent informations
+        # FIXME: should one day make use of git-config --section-remove,
+        # scheduled for 1.5.1
+        config.unset('branch.%s.remote' % self.get_name())
+        config.unset('branch.%s.merge' % self.get_name())
+        config.unset('branch.%s.stgit.parentbranch' % self.get_name())
+        config.unset('branch.%s.stgitformatversion' % self.get_name())
 
     def refresh_patch(self, files = None, message = None, edit = False,
                       show_patch = False,
@@ -735,7 +753,12 @@ class Series(StgitObject):
             committer_email = patch.get_commemail()
 
         if sign_str:
-            descr = '%s\n%s: %s <%s>\n' % (descr.rstrip(), sign_str,
+            descr = descr.rstrip()
+            if descr.find("\nSigned-off-by:") < 0 \
+               and descr.find("\nAcked-by:") < 0:
+                descr = descr + "\n"
+
+            descr = '%s\n%s: %s <%s>\n' % (descr, sign_str,
                                            committer_name, committer_email)
 
         bottom = patch.get_bottom()
@@ -792,21 +815,24 @@ class Series(StgitObject):
                   before_existing = False, refresh = True):
         """Creates a new patch
         """
-        self.__patch_name_valid(name)
 
-        if self.patch_applied(name) or self.patch_unapplied(name):
-            raise StackException, 'Patch "%s" already exists' % name
+        if name != None:
+            self.__patch_name_valid(name)
+            if self.patch_exists(name):
+                raise StackException, 'Patch "%s" already exists' % name
 
         if not message and can_edit:
-            descr = edit_file(self, None, \
-                              'Please enter the description for patch "%s" ' \
-                              'above.' % name, show_patch)
+            descr = edit_file(
+                self, None,
+                'Please enter the description for the patch above.',
+                show_patch)
         else:
             descr = message
 
         head = git.get_head()
 
-        self.__begin_stack_check()
+        if name == None:
+            name = make_patch_name(descr, self.patch_exists)
 
         patch = Patch(name, self.__patch_dir, self.__refs_dir)
         patch.create()
@@ -831,22 +857,18 @@ class Series(StgitObject):
             self.log_patch(patch, 'new')
 
             patches = [patch.get_name()] + self.get_unapplied()
-
-            f = file(self.__unapplied_file, 'w+')
-            f.writelines([line + '\n' for line in patches])
-            f.close()
+            write_strings(self.__unapplied_file, patches)
         elif before_existing:
             self.log_patch(patch, 'new')
 
             insert_string(self.__applied_file, patch.get_name())
-            if not self.get_current():
-                self.__set_current(name)
         else:
             append_string(self.__applied_file, patch.get_name())
-            self.__set_current(name)
             if refresh:
                 self.refresh_patch(cache_update = False, log = 'new')
 
+        return patch
+
     def delete_patch(self, name):
         """Deletes a patch
         """
@@ -868,14 +890,7 @@ class Series(StgitObject):
 
         unapplied = self.get_unapplied()
         unapplied.remove(name)
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
-
-        if self.patch_hidden(name):
-            self.unhide_patch(name)
-
-        self.__begin_stack_check()
+        write_strings(self.__unapplied_file, unapplied)
 
     def forward_patches(self, names):
         """Try to fast-forward an array of patches.
@@ -884,7 +899,6 @@ class Series(StgitObject):
         stack. Apply the rest with push_patch
         """
         unapplied = self.get_unapplied()
-        self.__begin_stack_check()
 
         forwarded = 0
         top = git.get_head()
@@ -948,12 +962,7 @@ class Series(StgitObject):
         git.switch(top)
 
         append_strings(self.__applied_file, names[0:forwarded])
-
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
-
-        self.__set_current(name)
+        write_strings(self.__unapplied_file, unapplied)
 
         return forwarded
 
@@ -983,8 +992,6 @@ class Series(StgitObject):
         unapplied = self.get_unapplied()
         assert(name in unapplied)
 
-        self.__begin_stack_check()
-
         patch = Patch(name, self.__patch_dir, self.__refs_dir)
 
         head = git.get_head()
@@ -1026,18 +1033,14 @@ class Series(StgitObject):
                 try:
                     git.merge(bottom, head, top, recursive = True)
                 except git.GitException, ex:
-                    print >> sys.stderr, \
-                          'The merge failed during "push". ' \
-                          'Use "refresh" after fixing the conflicts'
+                    out.error('The merge failed during "push".',
+                              'Use "refresh" after fixing the conflicts or'
+                              ' revert the operation with "push --undo".')
 
         append_string(self.__applied_file, name)
 
         unapplied.remove(name)
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
-
-        self.__set_current(name)
+        write_strings(self.__unapplied_file, unapplied)
 
         # head == bottom case doesn't need to refresh the patch
         if empty or head != bottom:
@@ -1091,12 +1094,13 @@ class Series(StgitObject):
 
         patch = Patch(name, self.__patch_dir, self.__refs_dir)
 
-        # only keep the local changes
-        if keep and not git.apply_diff(git.get_head(), patch.get_bottom()):
-            raise StackException, \
-                  'Failed to pop patches while preserving the local changes'
-
-        git.switch(patch.get_bottom(), keep)
+        if git.get_head_file() == self.get_name():
+            if keep and not git.apply_diff(git.get_head(), patch.get_bottom()):
+                raise StackException(
+                    'Failed to pop patches while preserving the local changes')
+            git.switch(patch.get_bottom(), keep)
+        else:
+            git.set_branch(self.get_name(), patch.get_bottom())
 
         # save the new applied list
         idx = applied.index(name) + 1
@@ -1104,24 +1108,11 @@ class Series(StgitObject):
         popped = applied[:idx]
         popped.reverse()
         unapplied = popped + self.get_unapplied()
-
-        f = file(self.__unapplied_file, 'w+')
-        f.writelines([line + '\n' for line in unapplied])
-        f.close()
+        write_strings(self.__unapplied_file, unapplied)
 
         del applied[:idx]
         applied.reverse()
-
-        f = file(self.__applied_file, 'w+')
-        f.writelines([line + '\n' for line in applied])
-        f.close()
-
-        if applied == []:
-            self.__set_current(None)
-        else:
-            self.__set_current(applied[-1])
-
-        self.__end_stack_check()
+        write_strings(self.__applied_file, applied)
 
     def empty_patch(self, name):
         """Returns True if the patch is empty
@@ -1151,27 +1142,15 @@ class Series(StgitObject):
         if newname in applied or newname in unapplied:
             raise StackException, 'Patch "%s" already exists' % newname
 
-        if self.patch_hidden(oldname):
-            self.unhide_patch(oldname)
-            self.hide_patch(newname)
-
         if oldname in unapplied:
             Patch(oldname, self.__patch_dir, self.__refs_dir).rename(newname)
             unapplied[unapplied.index(oldname)] = newname
-
-            f = file(self.__unapplied_file, 'w+')
-            f.writelines([line + '\n' for line in unapplied])
-            f.close()
+            write_strings(self.__unapplied_file, unapplied)
         elif oldname in applied:
             Patch(oldname, self.__patch_dir, self.__refs_dir).rename(newname)
-            if oldname == self.get_current():
-                self.__set_current(newname)
 
             applied[applied.index(oldname)] = newname
-
-            f = file(self.__applied_file, 'w+')
-            f.writelines([line + '\n' for line in applied])
-            f.close()
+            write_strings(self.__applied_file, applied)
         else:
             raise StackException, 'Unknown patch "%s"' % oldname
 
@@ -1195,24 +1174,39 @@ class Series(StgitObject):
     def hide_patch(self, name):
         """Add the patch to the hidden list.
         """
-        if not self.patch_exists(name):
-            raise StackException, 'Unknown patch "%s"' % name
-        elif self.patch_hidden(name):
-            raise StackException, 'Patch "%s" already hidden' % name
+        unapplied = self.get_unapplied()
+        if name not in unapplied:
+            # keep the checking order for backward compatibility with
+            # the old hidden patches functionality
+            if self.patch_applied(name):
+                raise StackException, 'Cannot hide applied patch "%s"' % name
+            elif self.patch_hidden(name):
+                raise StackException, 'Patch "%s" already hidden' % name
+            else:
+                raise StackException, 'Unknown patch "%s"' % name
+
+        if not self.patch_hidden(name):
+            # check needed for backward compatibility with the old
+            # hidden patches functionality
+            append_string(self.__hidden_file, name)
 
-        append_string(self.__hidden_file, name)
+        unapplied.remove(name)
+        write_strings(self.__unapplied_file, unapplied)
 
     def unhide_patch(self, name):
-        """Add the patch to the hidden list.
+        """Remove the patch from the hidden list.
         """
-        if not self.patch_exists(name):
-            raise StackException, 'Unknown patch "%s"' % name
         hidden = self.get_hidden()
         if not name in hidden:
-            raise StackException, 'Patch "%s" not hidden' % name
+            if self.patch_applied(name) or self.patch_unapplied(name):
+                raise StackException, 'Patch "%s" not hidden' % name
+            else:
+                raise StackException, 'Unknown patch "%s"' % name
 
         hidden.remove(name)
+        write_strings(self.__hidden_file, hidden)
 
-        f = file(self.__hidden_file, 'w+')
-        f.writelines([line + '\n' for line in hidden])
-        f.close()
+        if not self.patch_applied(name) and not self.patch_unapplied(name):
+            # check needed for backward compatibility with the old
+            # hidden patches functionality
+            append_string(self.__unapplied_file, name)