[only RFC, not merge] Second implementation of the "commit templates" feature

Adeodato Simó dato at net.com.org.es
Thu Aug 10 20:43:25 BST 2006


Hi all.

I'm attaching a patch with my second try to implement the commit
templates idea in bzr. It addresses the main concern Aaron expressed,
namely the need for a separate class for each minor variation, and now
the '--template foo,bar,baz' syntax proposed by John it's used.

I also did a careful reading of the through code reviews (thanks!), and
have tried not to make the same mistakes again (only John's comment
about StringIO/bzrlib.user_encoding() is left temporarily unaddressed, I
believe).

I'd like to here whether this second implementation looks cleaner, and
could make it into bzr (once polished!, and with tests¹). I'll also
probably want to wait until John answers about the generalized Factory
idea, and reimplement the functions in commit_templates.py using that,
before the current API would get into bzr.dev.

  (¹) I'm quite inexperienced wrt writing tests, so any suggestions as
      to what to test and/or how would be very welcome.

Cheers,

P.S.: The branch lives in http://people.debian.org/~adeodato/code/branches/bzr/commit_templates/.

-- 
Adeodato Simó                                     dato at net.com.org.es
Debian Developer                                  adeodato at debian.org
 
                              Listening to: Max Richter - Shadow Journal
-------------- next part --------------
revno: 1910
committer: Adeodato Simó <dato at net.com.org.es>
branch nick: commit_templates
timestamp: Thu 2006-08-10 21:22:26 +0200
message:
  Second version of the "commit templates" feature.

  Now, a commit file does not map to a class like before, which made it
  awkward to introduce small variations in the format. Instead, it is
  defined as an ordered list of "template bits".

  A "template bit" is any piece of information that can be included, to be
  implemented as a function with the signature:

    def template_bit_foo(working_tree, specific_files, to_file)

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2006-08-10 12:21:39 +0000
+++ bzrlib/builtins.py	2006-08-10 19:23:15 +0000
@@ -21,6 +21,7 @@
 import errno
 import os
 import os.path
+import re
 import sys
 
 import bzrlib
@@ -43,6 +44,7 @@
 from bzrlib.bundle.apply_bundle import install_bundle, merge_bundle
 from bzrlib.conflicts import ConflictList
 from bzrlib.commands import Command, display_command
+from bzrlib.commit_template import available_template_bits, CommitTemplateFile
 from bzrlib.errors import (BzrError, BzrCheckError, BzrCommandError, 
                            NotBranchError, DivergedBranches, NotConflicted,
                            NoSuchFile, NoWorkingTree, FileInWrongBranch,
@@ -1688,6 +1690,11 @@
                      Option('strict',
                             help="refuse to commit if there are unknown "
                             "files in the working tree."),
+                     Option('template', type=str, argname='LIST',
+                            help="comma separated list of the template bits to"
+                            " include in the commit file. Possible values are:"
+                            " %s." % (', '.join(available_template_bits()))
+                            ), # XXX: do --template help instead?
                      Option('local',
                             help="perform a local only commit in a bound "
                                  "branch. Such commits are not pushed to "
@@ -1698,12 +1705,11 @@
     aliases = ['ci', 'checkin']
 
     def run(self, message=None, file=None, verbose=True, selected_list=None,
-            unchanged=False, strict=False, local=False):
+            unchanged=False, strict=False, local=False, template=None):
         from bzrlib.commit import (NullCommitReporter, ReportCommitToLog)
         from bzrlib.errors import (PointlessCommit, ConflictsInTree,
                 StrictCommitFailed)
-        from bzrlib.msgeditor import edit_commit_message, \
-                make_commit_message_template
+        from bzrlib.msgeditor import edit_commit_message
         from tempfile import TemporaryFile
 
         # TODO: Need a blackbox test for invoking the external editor; may be
@@ -1724,8 +1730,10 @@
         if local and not tree.branch.get_bound_location():
             raise errors.LocalRequiresBoundBranch()
         if message is None and not file:
-            template = make_commit_message_template(tree, selected_list)
-            message = edit_commit_message(template)
+            template = CommitTemplateFile(tree, selected_list, template)
+            message = edit_commit_message(infotext=template.infotext(),
+                                          editable=template.editable())
+
             if message is None:
                 raise BzrCommandError("please specify a commit message"
                                       " with either --message or --file")

=== modified file 'bzrlib/msgeditor.py'
--- bzrlib/msgeditor.py	2006-07-31 13:11:43 +0000
+++ bzrlib/msgeditor.py	2006-08-10 19:16:49 +0000
@@ -26,6 +26,7 @@
 import bzrlib
 import bzrlib.config as config
 from bzrlib.errors import BzrError
+from bzrlib.symbol_versioning import deprecated_function, zero_ten
 from bzrlib.trace import warning, mutter
 
 
@@ -79,7 +80,7 @@
     { 'bar' : '-' * 14, 'msg' : 'This line and the following will be ignored' }
 
 
-def edit_commit_message(infotext, ignoreline=DEFAULT_IGNORE_LINE):
+def edit_commit_message(infotext, ignoreline=DEFAULT_IGNORE_LINE, editable=''):
     """Let the user edit a commit message in a temp file.
 
     This is run if they don't give a message or
@@ -89,6 +90,10 @@
         Text to be displayed at bottom of message for
         the user's reference; currently similar to
         'bzr status'.
+
+    editable:
+        Text to be displayed at the top of the message,
+        as a skeleton for the user to fill in.
     """
     import tempfile
 
@@ -98,12 +103,16 @@
         msgfile = os.close(tmp_fileno)
         if infotext is not None and infotext != "":
             hasinfo = True
+        else:
+            infotext = '\n'
+            hasinfo = False
+
+        if infotext or editable:
             msgfile = file(msgfilename, "w")
-            msgfile.write("\n\n%s\n\n%s" % (ignoreline,
-                infotext.encode(bzrlib.user_encoding, 'replace')))
+            parts = map(lambda s: s.encode(bzrlib.user_encoding, 'replace'),
+                    ( editable, ignoreline, infotext ))
+            msgfile.write("%s\n%s\n%s\n" % tuple(parts))
             msgfile.close()
-        else:
-            hasinfo = False
 
         if not _run_editor(msgfilename):
             return None
@@ -129,6 +138,8 @@
                 lastline = nlines
             msg.append(line)
             
+        # TODO check whether editable was left unmodified
+
         if len(msg) == 0:
             return ""
         # delete empty lines at the end
@@ -147,21 +158,8 @@
                 warning("failed to unlink %s: %s; ignored", msgfilename, e)
 
 
+ at deprecated_function(zero_ten)
 def make_commit_message_template(working_tree, specific_files):
-    """Prepare a template file for a commit into a branch.
-
-    Returns a unicode string containing the template.
-    """
-    # TODO: Should probably be given the WorkingTree not the branch
-    #
-    # TODO: make provision for this to be overridden or modified by a hook
-    #
-    # TODO: Rather than running the status command, should prepare a draft of
-    # the revision to be committed, then pause and ask the user to
-    # confirm/write a message.
-    from StringIO import StringIO       # must be unicode-safe
-    from bzrlib.status import show_tree_status
-    status_tmp = StringIO()
-    show_tree_status(working_tree, specific_files=specific_files, 
-                     to_file=status_tmp)
-    return status_tmp.getvalue()
+    """make_commit_message_template was deprecated in 0.10. Please see CommitTemplate."""
+    from commit_template import CommitTemplate
+    return CommitTemplate(working_tree, specific_files).infotext()

=== added file 'bzrlib/commit_template.py'
--- bzrlib/commit_template.py	1970-01-01 00:00:00 +0000
+++ bzrlib/commit_template.py	2006-08-10 19:14:45 +0000
@@ -0,0 +1,134 @@
+# Copyright (C) 2006 by Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+"""Template support for commit files."""
+
+
+import re
+from StringIO import StringIO
+
+from bzrlib.diff import show_diff_trees
+from bzrlib.errors import BzrCommandError
+from bzrlib.status import show_tree_status
+
+
+_marker = object()
+_commit_template_bits = {}
+
+
+class CommitTemplateFile(object):
+    """Class to represent a commit file."""
+
+    def __init__(self, working_tree, specific_files=[], template_bits=None):
+        """Create a new commit template file.
+
+        :param working_tree: Working tree where the commit is taking place.
+        :param specific_files: List of files that the user specified.
+        :param template_bits: List of template bits to include, either an
+            array or a comma-separated list; if None, config.commit_template()
+            is used.
+        """
+
+        self._editable = '\n'
+        self._infotext = '\n'
+
+        if template_bits is None:
+            template_bits = working_tree.branch.get_config().commit_template()
+
+        if isinstance(template_bits, basestring):
+            template_bits = re.split(r'[\s,]+', template_bits)
+
+        for bit in template_bits:
+            try:
+                function = get_commit_template(bit)
+            except KeyError:
+                raise BzrCommandError("Unknown template bit %s. Current known "
+                    "templates are: %s." % (bit,
+                        ', '.join(available_template_bits())))
+
+            if function.editable:
+                self._editable += function(working_tree, specific_files) + '\n'
+            else:
+                self._infotext += function(working_tree, specific_files) + '\n'
+
+    def editable(self):
+        """Return the editable part of the template."""
+        return self._editable
+
+    def infotext(self):
+        """Return the informational part of the template."""
+        return self._infotext
+
+
+class _CommitTemplateBit(object):
+    """Internal class to wrap a register'ed_commit_template() function."""
+
+    def __init__(self, name, function, editable):
+        self.name = name
+        self.function = function
+        self.editable = editable
+
+    def __call__(self, working_tree, specific_files):
+        to_file = StringIO()
+        if not self.editable:
+            to_file.write('# start %s\n' % self.name)
+        self.function(working_tree, specific_files, to_file)
+        return to_file.getvalue()
+
+
+def register_commit_template(name, function, editable=False):
+    """Registers a function as providing a certain template bit.
+
+    :param name: Name to register.
+    :param function: A function accepting (working_tree, specific_files,
+        to_file) as arguments. If present, its __doc__ will be shown in
+        'bzr commit --template help'. (TODO)
+    :param editable: A boolean telling whether the returned text is meant to
+        be placed above the ignore line.
+    """
+
+    assert callable(function)
+    _commit_template_bits[name] = _CommitTemplateBit(name, function, editable)
+
+
+def get_commit_template(name, default_value=_marker):
+    """Get a commit template bit by name.
+
+    Raises KeyError if the given name is not a registered template bit.
+    """
+
+    if default_value is _marker:
+        return _commit_template_bits[name]
+    else:
+        return _commit_template_bits.get(name, default_value)
+
+
+def available_template_bits():
+    """Returns a list of the registered template bits."""
+
+    return sorted(_commit_template_bits.keys())
+
+
+def _status(tree, files, to_file):
+    show_tree_status(tree, specific_files=files, to_file=to_file)
+
+
+def _diff(tree, files, to_file):
+    show_diff_trees(tree.basis_tree(), tree, to_file, files, None)
+
+
+register_commit_template('status', _status)
+register_commit_template('diff', _diff)

=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2006-08-07 19:25:24 +0000
+++ bzrlib/config.py	2006-08-10 19:18:26 +0000
@@ -28,6 +28,7 @@
 create_signatures=always|never|when-required(default)
 gpg_signing_command=name-of-program
 log_format=name-of-format
+commit_template=list-of-template-bits
 
 in locations.conf, you specify the url of a branch and options for it.
 Wildcards may be used - * and ? as normal in shell completion. Options
@@ -51,6 +52,9 @@
                     branch is configured to require them.
 log_format - this option sets the default log format.  Possible values are
              long, short, line, or a plugin can register new formats.
+commit_template - comma-separated list of template bits to include in commit
+                  message files. See "bzr commit --template help" for a list of
+                  possible values.
 
 In bazaar.conf you can also define aliases in the ALIASES sections, example
 
@@ -144,6 +148,17 @@
         """See log_format()."""
         return None
 
+    def commit_template(self):
+        """What template bits to include in commit files"""
+        result = self._commit_template()
+        if result is None:
+            result = "status"
+        return result
+
+    def _commit_template(self):
+        """See commit_template()."""
+        return None
+
     def __init__(self):
         super(Config, self).__init__()
 
@@ -291,6 +306,10 @@
         """See Config.log_format."""
         return self._get_user_option('log_format')
 
+    def _commit_template(self):
+        """See Config.commit_template."""
+        return self._get_user_option('commit_template')
+
     def __init__(self, get_filename):
         super(IniBasedConfig, self).__init__()
         self._get_filename = get_filename
@@ -565,6 +584,10 @@
         """See Config.log_format."""
         return self._get_best_value('_log_format')
 
+    def _commit_template(self):
+        """See Config.commit_template."""
+        return self._get_best_value('_commit_template')
+
 
 def ensure_config_dir_exists(path=None):
     """Make sure a configuration directory exists.



More information about the bazaar mailing list