Allow editing attachment comment and obsoletes

Add a -e/--edit option to 'git-bz-attach' to bring up the
description/comment in an editor. Existing patches are shown
as commented out Obsoletes: lines that can be uncommented to
to obsolete the old patches.

2 files changed, 104 insertions, 41 deletions

diff --git a/TODO b/TODO
index edb9cb5..575cc7c 100644
--- a/TODO
+++ b/TODO
@@ -25,18 +25,6 @@ Reconsider initial description for 'git bz file'
   - If the user leaves it empty use the body as the initial description
     as currently.
 
-Allow editing comment used for attachments
-
- When attaching a revised version of a patch, you really want to be
- able to edit the comment to say what has changed. 'attach' should
- support -e/--edit to do this.
-
- In the edit buffer would be commented out lines:
-
- # Obsoletes: 23212 - Frobinificate faster
-
- That you could uncomment to obsolete old patches.
-
 Use XML-RPC when available.
 
   Maybe use python-bugzilla: http://fedorahosted.org/python-bugzilla/
diff --git a/git-bz b/git-bz
index 5bf2826..389785f 100755
--- a/git-bz
+++ b/git-bz
@@ -65,6 +65,9 @@
 #
 #    Prompts before actually doing anything to avoid mistakes.
 #
+#    If -e/--edit is specified, then the user can edit the description and
+#    comment for each patch, and (by uncommenting lines) obsolete old patches.
+#
 #    Examples:
 #
 #       # Attach the last commit
@@ -494,7 +497,8 @@ def get_bugzilla_cookies(host):
 # Based on http://code.activestate.com/recipes/146306/ - Wade Leftwich
 def encode_multipart_formdata(fields, files):
     """
-    fields is a dictionary of { name : value } for regular form fields.
+    fields is a dictionary of { name : value } for regular form fields. if value is a list,
+      one form field is added for each item in the list
     files is a dictionary of { name : ( filename, content_type, value) } for data to be uploaded as files
     Return (content_type, body) ready for httplib.HTTPContent instance
     """
@@ -503,10 +507,17 @@ def encode_multipart_formdata(fields, files):
     L = []
     for key in sorted(fields.keys()):
         value = fields[key]
-        L.append('--' + BOUNDARY)
-        L.append('Content-Disposition: form-data; name="%s"' % key)
-        L.append('')
-        L.append(value)
+        if isinstance(value, list):
+            for v in value:
+                L.append('--' + BOUNDARY)
+                L.append('Content-Disposition: form-data; name="%s"' % key)
+                L.append('')
+                L.append(v)
+        else:
+            L.append('--' + BOUNDARY)
+            L.append('Content-Disposition: form-data; name="%s"' % key)
+            L.append('')
+            L.append(value)
     for key in sorted(files.keys()):
         (filename, content_type, value) = files[key]
         L.append('--' + BOUNDARY)
@@ -530,7 +541,7 @@ def make_filename(description):
 
     return filename
 
-def edit(filename):
+def edit_file(filename):
     editor = None
     if 'GIT_EDITOR' in os.environ:
         editor = os.environ['GIT_EDITOR']
@@ -549,6 +560,36 @@ def edit(filename):
     if process.returncode != 0:
         die("Editor exited with non-zero return code")
 
+def edit_template(template):
+    # Prompts the user to edit the text 'template' and returns list of
+    # lines with comments stripped
+
+    handle, filename = tempfile.mkstemp(".txt", "git-bz-")
+    f = os.fdopen(handle, "w")
+    f.write(template)
+    f.close()
+
+    edit_file(filename)
+
+    f = open(filename, "r")
+    lines = filter(lambda x: not x.startswith("#"), f.readlines())
+    f.close
+
+    return lines
+
+def split_subject_body(lines):
+    # Splits the first line (subject) from the subsequent lines (body)
+
+    i = 0
+    subject = ""
+    while i < len(lines):
+        subject = lines[i].strip()
+        if subject != "":
+            break
+        i += 1
+
+    return subject, "".join(lines[i + 1:]).strip()
+
 def prompt(message):
     print message, "[yn] ",
     line = sys.stdin.readline().strip()
@@ -653,7 +694,7 @@ class Bug(object):
         print "Bug %d - %s" % (self.id, short_desc)
         print self.get_url()
 
-    def create_patch(self, description, comment, filename, data):
+    def create_patch(self, description, comment, filename, data, obsoletes=[]):
         fields = {}
         fields['bugid'] = str(self.id)
         fields['action'] = 'insert'
@@ -661,6 +702,10 @@ class Bug(object):
         fields['description'] = description
         if comment:
             fields['comment'] = comment
+        if obsoletes:
+            # this will produce multiple parts in the encoded data with the
+            # name 'obsolete' for each item in the list
+            fields['obsolete'] = map(str, obsoletes)
 
         files = {}
         files['data'] = (filename, 'text/plain', data)
@@ -831,7 +876,44 @@ def strip_bug_url(bug, commit_body):
     pattern = "\s*" + re.escape(bug.get_url()) + "\s*$"
     return re.sub(pattern, "", commit_body)
 
-def attach_commits(bug, commits, include_comments=True):
+def edit_attachment_comment(bug, initial_description, initial_body):
+    template = StringIO()
+    template.write(initial_description)
+    template.write("\n\n")
+    template.write(initial_body)
+    template.write("\n\n")
+    if len(bug.patches) > 0:
+        for patch in bug.patches:
+            template.write("#Obsoletes: %d -%s\n" % (patch.attach_id, patch.description))
+        template.write("\n")
+
+    template.write("""# Please edit the description (first line) and comment (other lines). Lines
+# starting with '#' will be ignored.  Delete everything to abort.
+""")
+    if len(bug.patches) > 0:
+        template.write("# To obsolete existing patches, uncomment the appropriate lines.\n")
+
+    lines = edit_template(template.getvalue())
+
+    obsoletes= []
+    def filter_obsolete(line):
+        m = re.match("^\s*Obsoletes\s*:\s*([\d]+)", line)
+        if m:
+            obsoletes.append(int(m.group(1)))
+            return False
+        else:
+            return True
+
+    lines = filter(filter_obsolete, lines)
+
+    description, comment = split_subject_body(lines)
+
+    if description == "":
+        die("Empty description, aborting")
+
+    return description, comment, obsoletes
+
+def attach_commits(bug, commits, include_comments=True, edit_comments=False):
     # We want to attach the patches in chronological order
     commits = list(commits)
     commits.reverse()
@@ -843,7 +925,12 @@ def attach_commits(bug, commits, include_comments=True):
             body = strip_bug_url(bug, get_body(commit))
         else:
             body = None
-        bug.create_patch(commit.subject, body, filename, patch)
+        if edit_comments:
+            description, body, obsoletes = edit_attachment_comment(bug, commit.subject, body)
+        else:
+            description = commit.subject
+            obsoletes = []
+        bug.create_patch(commit.subject, body, filename, patch, obsoletes=obsoletes)
 
 def do_attach(bug_reference, since_or_revision_range):
     commits = get_commits(since_or_revision_range)
@@ -866,7 +953,7 @@ def do_attach(bug_reference, since_or_revision_range):
     if global_options.add_url:
         add_url(bug, commits)
 
-    attach_commits(bug, commits)
+    attach_commits(bug, commits, edit_comments=global_options.edit)
 
 def do_file(*args):
     if len(args) == 1:
@@ -919,30 +1006,13 @@ def do_file(*args):
     for commit in commits:
         template.write("#   " + commit.id[0:7] + " " + commit.subject + "\n")
 
-    handle, filename = tempfile.mkstemp(".txt", "git-bz-")
-    f = os.fdopen(handle, "w")
-    f.write(template.getvalue())
-    f.close()
-
-    edit(filename)
-
-    f = open(filename, "r")
-    lines = filter(lambda x: not x.startswith("#"), f.readlines())
-    f.close()
+    lines = edit_template(template.getvalue())
 
-    i = 0
-    summary = ""
-    while i < len(lines):
-        summary = lines[i].strip()
-        if summary != "":
-            break
-        i += 1
+    summary, description = split_subject_body(lines)
 
     if summary == "":
         die("Empty summary, aborting")
 
-    description = "".join(lines[i + 1:]).strip()
-
     bug = Bug.create(get_tracker(), product, component, summary, description)
 
     if global_options.add_url:
@@ -975,6 +1045,10 @@ def add_add_url_option():
     parser.add_option("-u", "--add-url", action="store_true",
                       help="rewrite commits to add the bug URL")
 
+def add_edit_option():
+    parser.add_option("-e", "--edit", action="store_true",
+                      help="allow editing the bugzilla comment")
+
 if command == 'add-url':
     parser.set_usage("git bz add-url [-<N>] [options] <bug reference> [<since | <revision range>]");
     add_num_option()
@@ -987,6 +1061,7 @@ elif command == 'attach':
     parser.set_usage("git bz attach [-<N>] [options] <bug reference> [<since | <revision range>]");
     add_add_url_option()
     add_num_option()
+    add_edit_option()
     min_args = max_args = 2
 elif command == 'file':
     parser.set_usage("git bz file [-<N>] [options] <product>/<component> [<since> | <revision range>]");