Merge pull request #32 from codebykat/changeset-comments

Add line-level comments on changesets

Author: nb

README.md

@@ -30,6 +30,8 @@ in context, below the line in question.
 
 * Comments on changesets – useful when doing code reviews of incoming commits.
 
+* Inline comments on changesets - comment on a specific line of the changeset.
+
 * Comments on attachment pages – useful when reviewing patches.
 
 * Wiki Markup – you can use the standard Trac wiki markup inside your
@@ -70,11 +72,11 @@ Roadmap
 
 Nobody can predict the future, but here are some features on the roadmap:
 
-* Line-level comments for changesets and diff attachments, too
+* Line-level comments for diff attachments, too
 * E-mail notifications
 
 License
 -------
 Copyright (C) 2011-2012, Automattic Inc.
 
-This plugin is distributed under the GPLv2 or later license.
+This plugin is distributed under the GPLv2 or later license.

code_comments/comment.py

@@ -38,15 +38,17 @@ def __init__(self, req, env, data):
         self.req = req
         if self._empty('version'):
             self.version = VERSION
+        if self._empty( 'path' ):
+            self.path = ''
         self.html = format_to_html(self.req, self.env, self.text)
         email = self.email_map().get(self.author, 'baba@baba.net')
         self.email_md5 = md5_hexdigest(email)
         attachment_info = self.attachment_info()
-        self.is_comment_to_attachment = attachment_info['is']
+        self.is_comment_to_attachment = 'attachment' == self.type
         self.attachment_ticket = attachment_info['ticket']
         self.attachment_filename = attachment_info['filename']
-        self.is_comment_to_changeset = self.revision and not self.path
-        self.is_comment_to_file = self.revision and self.path
+        self.is_comment_to_changeset = 'changeset' == self.type
+        self.is_comment_to_file = 'browser' == self.type
 
     def _empty(self, column_name):
         return not hasattr(self, column_name) or not getattr(self, column_name)
@@ -71,25 +73,31 @@ def href(self):
             href = self.req.href.changeset(self.revision, codecomment=self.id)
         elif self.is_comment_to_attachment:
             href = self.req.href('/attachment/ticket/%d/%s' % (self.attachment_ticket, self.attachment_filename), codecomment=self.id)
-        if self.line:
+        if self.line and not self.is_comment_to_changeset:
             href += '#L' + str(self.line)
         return href
 
     def link_text(self):
-        if self.revision and not self.path:
-            return '[%s]' % self.revision
-        if self.path.startswith('attachment:'):
+        if self.is_comment_to_changeset:
+            return self.changeset_link_text()
+        if self.is_comment_to_attachment:
             return self.attachment_link_text()
 
-        # except the two specials cases of changesets (revision-only)
-        # and arrachments (path-only), we must always have them both
+        # except the two special cases of changesets (revision-only)
+        # and attachments (path-only), we must always have them both
         assert self.path and self.revision
 
         link_text = self.path + '@' + str(self.revision)
         if self.line:
             link_text += '#L' + str(self.line)
         return link_text
 
+    def changeset_link_text(self):
+        if 0 != self.line:
+            return 'Changeset @%d#L%d (in %s)' % ( self.revision, self.line, self.path )
+        else:
+            return 'Changeset @%s' % self.revision
+
     def attachment_link_text(self):
         return '#%s: %s' % (self.attachment_ticket, self.attachment_filename)
 
@@ -99,9 +107,8 @@ def trac_link(self):
         return 'source:' + self.link_text()
 
     def attachment_info(self):
-        info = {'is': False, 'ticket': None, 'filename': None}
-        info['is'] = self.path.startswith('attachment:')
-        if not info['is']:
+        info = { 'ticket': None, 'filename': None }
+        if not self.path.startswith( 'attachment' ):
             return info
         match = re.match(r'attachment:/ticket/(\d+)/(.*)', self.path)
         if not match:

code_comments/db.py

@@ -4,7 +4,7 @@
 from trac.db.api import DatabaseManager
 
 # Database version identifier for upgrades.
-db_version = 1
+db_version = 2
 
 # Database schema
 schema = {
@@ -17,6 +17,7 @@
         Column('line', type='int'),
         Column('author'),
         Column('time', type='int'),
+        Column('type'),
         Index(['path']),
         Index(['author']),
     ],
@@ -34,15 +35,48 @@ def create_tables(env, db):
             cursor.execute(stmt)
     cursor.execute("INSERT into system values ('code_comments_schema_version', %s)",
                         str(db_version))
+
 # Upgrades
 def upgrade_from_1_to_2(env, db):
-    pass
+    # Add the new column "type"
+    @env.with_transaction()
+    def add_type_column( db ):
+        cursor = db.cursor()
+        cursor.execute( 'ALTER TABLE code_comments ADD COLUMN type TEXT' )
+
+    # Convert all the current comments to the new schema
+    @env.with_transaction()
+    def convert_comments( db ):
+        comments = {}
+        cursor = db.cursor()
+        cursor.execute( 'SELECT id, path FROM code_comments' )
+        comments = cursor.fetchall()
+        # options:
+        # 1: comment on file (path != "" && path != "attachment")
+        # 2: comment on changeset (path == "")
+        # 3: comment on attachment (path == "attachment")
+        for comment in comments:
+            path = comment[1]
+            is_comment_to_attachment = path.startswith( 'attachment' )
+            is_comment_to_file = not is_comment_to_attachment and '' != path
+            is_comment_to_changeset = '' == path
+            cursor = db.cursor()
+            update = 'UPDATE code_comments SET type={0} WHERE id={1}'
+            sql = ''
+
+            if is_comment_to_changeset:
+                sql = update.format( "'changeset'", str( comment[0] ) )
+            elif is_comment_to_attachment:
+                sql = update.format( "'attachment'", str(comment[0] ) )
+            elif is_comment_to_file:
+                sql = update.format( "'browser'", str(comment[0] ) )
+
+            cursor.execute( sql )
 
 upgrade_map = {
         2: upgrade_from_1_to_2
     }
 
-
 class CodeCommentsSetup(Component):
     """Component that deals with database setup and upgrades."""
 

code_comments/htdocs/code-comments.css

@@ -69,12 +69,30 @@ tr.with-comments {
 	background-color: #FFFFE2;
 	border-bottom: 1px solid #998;
 }
-tr.comments {
+tr.comments td {
 	border-bottom: 1px solid #998;
 }
-table.code tr {
+/* fixes .with-comments bottom borders on changeset view */
+.diff table.inline tbody.mod tr.with-comments td.r,
+.diff table.inline tbody.add tr.with-comments td.r {
+	border-bottom-width: 1px;
+}
+.diff table.inline tbody.mod tr.with-comments td.l,
+.diff table.inline tbody.rem tr.with-comments td.l {
+	border-bottom-width: 1px;
+}
+
+table.code tr,
+.trac-diff tbody tr {
 	height: 17px;
 }
-table.code tr:hover td {
+
+table.code tr:hover td,
+#content table.trac-diff tbody tr:hover td {
 	background-color: #FFFFCA;
+}
+
+a.bubble span.ui-icon {
+	/* this centers the icon horizontally and vertically */
+	margin: -2px auto 0;
 }