WIP: support inline comment threads

Change-Id: I570c4a85953511c1f5d74653df9d2c4aae45a474
This commit is contained in:
James E. Blair 2022-10-06 09:04:38 -07:00
parent 559bcfdb33
commit e10b81872b
6 changed files with 423 additions and 32 deletions

View File

@ -138,6 +138,7 @@ comment_table = Table(
Column('line', Integer),
Column('message', Text, nullable=False),
Column('draft', Boolean, index=True, nullable=False),
Column('unresolved', Boolean, index=True, nullable=False),
Column('robot_id', String(255)),
Column('robot_run_id', String(255)),
Column('url', Text()),
@ -317,6 +318,32 @@ class Topic(object):
self.projects.remove(project)
session.flush()
class Thread:
def __init__(self):
self.origin_id = None
self.ids = set()
self.key = None
self.comments = []
self.unresolved = False
self.patchset = None
self.path = None
self.line = None
self.has_draft = False
def addComment(self, path, ps, message, comment):
self.unresolved = comment.unresolved
if comment.id:
self.ids.add(comment.id)
self.comments.append(comment)
if self.key is None:
self.origin_id = comment.id
self.key = (path, ps, comment.line or 0, comment.message)
self.line = comment.line or ''
self.patchset = ps
self.path = path
if comment.draft:
self.has_draft = True
class Change(object):
def __init__(self, project, id, owner, number, branch, change_id,
subject, created, updated, status, topic=None,
@ -361,6 +388,38 @@ class Change(object):
self._updateApprovalCache()
return self._approval_cache.get(category, 0)
def getThreads(self):
if not hasattr(self, '_threads'):
self.makeThreads()
return self._threads
def makeThreads(self):
threads = []
messages = {}
for message in self.messages:
messages[(message.author.id, message.created)] = message
for revno, revision in enumerate(self.revisions):
for file in revision.files:
for comment in file.comments:
message = messages.get((comment.author.id, comment.created))
if not message:
continue
comment_ps = revno + 1
thread = None
for t in threads:
if comment.in_reply_to in t.ids:
thread = t
if not thread:
thread = Thread()
threads.append(thread)
thread.addComment(comment.file.path, comment_ps, message, comment)
comment._thread = thread
comment._message = message
threads.sort(key=lambda t: t.key)
self._threads = threads
def _updateApprovalCache(self):
cat_min = {}
cat_max = {}
@ -586,7 +645,7 @@ class Message(object):
class Comment(object):
def __init__(self, file, id, author, in_reply_to, created, parent, line, message, draft=False,
robot_id=None, robot_run_id=None, url=None):
robot_id=None, robot_run_id=None, url=None, unresolved=False):
self.file_key = file.key
self.account_key = author.key
self.id = id
@ -599,6 +658,7 @@ class Comment(object):
self.robot_id = robot_id
self.robot_run_id = robot_run_id
self.url = url
self.unresolved = unresolved
class Label(object):
def __init__(self, change, category, value, description):

View File

@ -63,6 +63,23 @@ DEFAULT_PALETTE={
'focused-revision-commit': ['dark blue,standout', ''],
'focused-revision-comments': ['default,standout', ''],
'focused-revision-drafts': ['dark red,standout', ''],
'comment-thread-unresolved': ['yellow', ''],
'comment-thread-unresolved-name': ['yellow', ''],
'comment-thread-unresolved-header': ['brown', ''],
'comment-thread-unresolved-own-name': ['light cyan', ''],
'comment-thread-unresolved-own-header': ['dark cyan', ''],
'comment-thread-resolved': ['dark gray', ''],
'comment-thread-resolved-name': ['dark gray', ''],
'comment-thread-resolved-header': ['dark gray', ''],
'comment-thread-resolved-own-name': ['dark gray', ''],
'comment-thread-resolved-own-header': ['dark gray', ''],
'comment-thread-checkbox': ['dark magenta', ''],
'focused-comment-thread-checkbox': ['light magenta', ''],
'comment-thread-button': ['dark magenta', ''],
'focused-comment-thread-button': ['light magenta', ''],
'change-message-name': ['yellow', ''],
'change-message-own-name': ['light cyan', ''],
'change-message-header': ['brown', ''],

View File

@ -798,6 +798,10 @@ class SyncChangeTask(Task):
username=remote_comment['author'].get('username'),
email=remote_comment['author'].get('email'))
comment = session.getCommentByID(remote_comment['id'])
# Unresolved can't really change, but we may
# have old data before we started storing
# unresolved.
unresolved = remote_comment.get('unresolved', False)
if not comment:
# Normalize updated -> created
created = dateutil.parser.parse(remote_comment['updated'])
@ -814,12 +818,15 @@ class SyncChangeTask(Task):
remote_comment['message'],
robot_id = remote_comment.get('robot_id'),
robot_run_id = remote_comment.get('robot_run_id'),
url = remote_comment.get('url'))
url = remote_comment.get('url'),
unresolved = unresolved)
self.log.info("Created new comment %s for revision %s in local DB.",
comment.key, revision.key)
else:
if comment.author != account:
comment.author = account
if comment.unresolved != unresolved:
comment.unresolved = unresolved
if remote_revision.get('_gertty_remote_checks_data'):
self._updateChecks(session, change, revision, remote_revision['_gertty_remote_checks_data'])
# End revisions
@ -1391,9 +1398,12 @@ class UploadReviewTask(Task):
comment_list = []
for comment in file.draft_comments:
d = dict(line=comment.line,
unresolved=comment.unresolved,
message=comment.message)
if comment.parent:
d['side'] = 'PARENT'
if comment.in_reply_to:
d['in_reply_to'] = comment.in_reply_to
comment_list.append(d)
session.delete(comment)
comments[file.path] = comment_list

View File

@ -394,6 +394,230 @@ class ChangeButton(urwid.Button):
except gertty.view.DisplayError as e:
self.change_view.app.error(e.message)
class ThreadCommentBox(mywid.HyperText):
def __init__(self, change_view, change, thread, message, comment):
super().__init__('')
self.change_view = change_view
self.app = change_view.app
self.refresh(change, thread, message, comment)
def selectable(self):
return len(self.selectable_items) > 0
def refresh(self, change, thread, message, comment):
self.comment_key = comment.key
self.message_created = message.created
self.message_author = message.author_name
self.message_text = message.message
created = self.app.time(message.created)
unresolved = thread.unresolved
if thread.has_draft:
unresolved = True
if unresolved:
style = 'comment-thread-unresolved'
else:
style = 'comment-thread-resolved'
if self.app.isOwnAccount(message.author):
name_style = f'{style}-own-name'
header_style = f'{style}-own-header'
reviewer_string = message.author_name
else:
name_style = f'{style}-name'
header_style = f'{style}-header'
if message.author.email:
reviewer_string = "%s <%s>" % (
message.author_name,
message.author.email)
else:
reviewer_string = message.author_name
text = [(name_style, reviewer_string),
(header_style, f': Patch Set {message.revision.number}'),
(header_style, created.strftime(' (%Y-%m-%d %H:%M:%S%z)'))]
if message.draft and not message.pending:
text.append(('change-message-draft', ' (draft)'))
#comment_text = ['\n'.join(lines)]
#for commentlink in self.app.config.commentlinks:
# comment_text = commentlink.run(self.app, comment_text)
text.append('\n')
text.append(comment.message)
self.set_text(text)
class ThreadCommentEdit(urwid.WidgetWrap):
def __init__(self, app, thread):
super().__init__(urwid.Pile([]))
self.pile = urwid.Pile([])
self._w = urwid.AttrMap(self.pile, None, focus_map={})
self.app = app
# If we save a comment, the resulting key will be stored here
self.key = None
self.comment = mywid.MyEdit(edit_text='', multiline=True,
ring=self.app.ring)
#comment = urwid.Padding(self.comment, width=80)
self.checkbox = urwid.CheckBox('Resolved',
state=not thread.unresolved)
self.pile.contents.append((urwid.AttrMap(self.comment, 'draft-comment'),
('pack', None)))
self.pile.contents.append((urwid.AttrMap(self.checkbox,
'comment-thread-checkbox',
'focused-comment-thread-checkbox'),
('pack', None)))
#self.focus_position = 1
class ThreadBox(urwid.WidgetWrap):
focus_map={'comment-thread-button': 'focused-comment-thread-button'}
def __init__(self, change_view, change, thread):
super().__init__(urwid.Pile([]))
self.pile = urwid.Pile([])
self._w = urwid.AttrMap(self.pile, None, focus_map={})
self.change_view = change_view
self.app = change_view.app
self.edit = None
def x(self, button):
pass
def deleteComment(self, comment_key):
self.app.log.debug("delete comment %s", comment_key)
with self.app.db.getSession() as session:
comment = session.getComment(comment_key)
session.delete(comment)
def saveComment(self, text):
orig_comment = self.thread.comments[0]
orig_message = orig_comment._message
message_key = None
in_reply_to = orig_comment.id
with self.app.db.getSession() as session:
account = session.getOwnAccount()
revision = session.getRevision(orig_message.revision_key)
draft_message = revision.getPendingMessage()
now = datetime.datetime.utcnow()
if not draft_message:
draft_message = revision.getDraftMessage()
if not draft_message:
draft_message = revision.createMessage(None, account,
now,
'', draft=True)
draft_message.created = now
message_key = draft_message.key
fileobj = session.getFile(orig_comment.file_key)
comment = fileobj.createComment(None, account, in_reply_to,
now,
orig_comment.parent,
orig_comment.line, text, draft=True)
key = comment.key
self.app.log.debug("save comment %s %s", text, key)
return key
def cleanupEdit(self):
edit = self.edit
if not edit:
return
text = edit.comment.edit_text.strip()
if text:
if edit.key:
with self.app.db.getSession() as session:
comment = session.getComment(edit.key)
comment.message = text
comment.unresolved = not edit.checkbox.get_state()
else:
edit.key = self.saveComment(text)
else:
if edit.key:
self.deleteComment(edit.key)
edit.key = None
self.closeEdit()
self.app.log.debug("XXX close edit %s", self.thread.comments)
self.change_view.refresh()
def reply(self, button):
self.openEdit()
self.pile.set_focus(len(self.pile.contents)-3)
def _addEdit(self):
row = urwid.Padding(self.edit, width=80)
self.pile.contents.insert(-2,
(row, ('pack', None)))
def openEdit(self):
if self.edit: return False
self.edit = ThreadCommentEdit(self.app, self.thread)
self._addEdit()
return True
def closeEdit(self):
if not self.edit: return
self.edit = None
self.pile.contents.pop(-3)
def refresh(self, change, thread):
self.thread = thread
self.pile.contents = []
path = thread.path
if path == '/PATCHSET_LEVEL':
path = ''
if path == '/COMMIT_MSG':
path = 'Commit message'
location = f'Patchset {thread.patchset} {path} {thread.line}'
unresolved = thread.unresolved
if thread.has_draft:
unresolved = True
if unresolved:
style = 'comment-thread-unresolved'
else:
style = 'comment-thread-resolved'
self.pile.contents.append((urwid.Text((style, location)), ('pack', None)))
draft_comment = None
#self.listbox.body.append('thread')
listbox_index = 0
for comment in thread.comments:
message = comment._message
self.app.log.debug("%s %s", message, comment.message)
if comment.draft:
draft_comment = comment
else:
box = ThreadCommentBox(self, change, thread, message, comment)
row = urwid.Padding(box, width=80)
self.pile.contents.append((row, ('pack', None)))
#self.message_rows[message.key] = row
buttons = [
mywid.FixedButton(('comment-thread-button', "Reply"),
on_press=self.reply),
mywid.FixedButton(('comment-thread-button', "Quote"),
on_press=self.x),
]
if unresolved:
buttons.append(
mywid.FixedButton(('comment-thread-button', "Ack"),
on_press=self.x))
buttons.append(
mywid.FixedButton(('comment-thread-button', "Done"),
on_press=self.x))
buttons = [('pack', urwid.AttrMap(b, None, focus_map=self.focus_map)) for b in buttons]
buttons = urwid.Columns(buttons + [urwid.Text('')], dividechars=2)
buttons = urwid.AttrMap(buttons, 'comment-thread-button')
self.pile.contents.append((buttons, ('pack', None)))
self.pile.contents.append((urwid.Text(''), ('pack', None)))
if draft_comment:
if not self.openEdit():
self._addEdit()
self.edit.comment.set_edit_text(draft_comment.message)
self.edit.key = draft_comment.key
self.pile.set_focus(len(self.pile.contents)-3)
else:
self.pile.set_focus(len(self.pile.contents)-2)
class ChangeMessageBox(mywid.HyperText):
def __init__(self, change_view, change, message):
super(ChangeMessageBox, self).__init__(u'')
@ -495,19 +719,22 @@ class ChangeMessageBox(mywid.HyperText):
comment_ps = revno + 1
if comment_ps == message.revision.number:
comment_ps = None
inline_comments[path].append((comment_ps or 0, comment.line or 0, comment.message))
comment_key = (comment_ps or 0, comment.line or 0, comment.message)
inline_comments[path].append((comment_key, comment))
for v in inline_comments.values():
v.sort()
# Sort on comment_key
v.sort(key=lambda x: x[0])
if inline_comments:
comment_text.append(u'\n')
for key, value in inline_comments.items():
if key == '/PATCHSET_LEVEL':
key = 'Patchset'
if key == '/COMMIT_MSG':
key = 'Commit message'
comment_text.append(('filename-inline-comment', u'%s' % key))
for patchset, line, comment in value:
for path, comment_list in inline_comments.items():
if path == '/PATCHSET_LEVEL':
path = 'Patchset'
if path == '/COMMIT_MSG':
path = 'Commit message'
comment_text.append(('filename-inline-comment', u'%s' % path))
for comment_key, comment in comment_list:
patchset, line, message = comment_key
location_str = ''
if patchset:
location_str += "PS%i" % patchset
@ -516,8 +743,9 @@ class ChangeMessageBox(mywid.HyperText):
location_str += str(line)
if location_str:
location_str += ": "
comment_text.append(u'\n %s%s\n' % (location_str, comment))
comment_text.append(u'\n %s%s\n' % (location_str, message))
comment_text.append(f'{comment.unresolved}\n')
self.app.log.debug(f'unresolved: {comment.unresolved}\n')
self.set_text(text+comment_text)
class CommitMessageBox(mywid.HyperText):
@ -597,6 +825,7 @@ class ChangeView(urwid.WidgetWrap):
self.log = logging.getLogger('gertty.view.change')
self.app = app
self.change_key = change_key
self.thread_rows = {}
self.revision_rows = {}
self.message_rows = {}
self.last_revision_key = None
@ -838,6 +1067,25 @@ class ChangeView(urwid.WidgetWrap):
if len(self.listbox.body) == listbox_index:
self.listbox.body.insert(listbox_index, urwid.Divider())
listbox_index += 1
# XXX
unseen_keys = set(self.thread_rows.keys())
for thread in change.getThreads():
self.app.log.debug("%s %s", thread, thread.unresolved)
row = self.thread_rows.get(thread.key)
if not row:
row = ThreadBox(self, change, thread)
self.listbox.body.insert(listbox_index, row)
self.thread_rows[thread.key] = row
row.refresh(change, thread)
listbox_index += 1
unseen_keys.discard(thread.key)
for key in unseen_keys:
row = self.thread_rows.get(key)
self.listbox.body.remove(row)
del self.thread_rows[key]
listbox_index -= 1
# Get the set of messages that should be displayed
display_messages = []
result_systems = {}
@ -1006,10 +1254,18 @@ class ChangeView(urwid.WidgetWrap):
return self.app.toggleHeldChange(self.change_key)
def keypress(self, size, key):
old_focus = self.listbox.focus
if not self.app.input_buffer:
key = super(ChangeView, self).keypress(size, key)
new_focus = self.listbox.focus
keys = self.app.input_buffer + [key]
commands = self.app.config.keymap.getCommands(keys)
if (isinstance(old_focus, ThreadBox) and
(old_focus != new_focus or (keymap.PREV_SCREEN in commands))):
old_focus.cleanupEdit()
if keymap.TOGGLE_REVIEWED in commands:
self.toggleReviewed()
self.refresh()

View File

@ -100,7 +100,7 @@ class BaseDiffCommentEdit(urwid.Columns):
pass
class BaseDiffComment(urwid.Columns):
pass
focus_map={'comment-thread-button': 'focused-comment-thread-button'}
class BaseDiffLine(urwid.Button):
def selectable(self):
@ -193,6 +193,7 @@ class BaseDiffView(urwid.WidgetWrap, mywid.Searchable):
self.searchInit()
with self.app.db.getSession() as session:
new_revision = session.getRevision(self.new_revision_key)
new_revision.change.makeThreads()
old_comments = []
new_comments = []
self.old_file_keys = {}
@ -251,12 +252,12 @@ class BaseDiffView(urwid.WidgetWrap, mywid.Searchable):
key += '-' + str(comment.line)
key += '-' + path
comment_list = comment_lists.get(key, [])
if comment.draft:
message = comment.message
else:
message = [('comment-name', comment.author.name),
('comment', u': '+comment.message)]
comment_list.append((comment.key, message))
#if comment.draft:
# message = comment.message
#else:
# message = [('comment-name', comment.author.name),
# ('comment', u': '+comment.message)]
comment_list.append((comment.key, comment))
comment_lists[key] = comment_list
comment_filenames.add(path)
for comment in old_comments:
@ -271,12 +272,12 @@ class BaseDiffView(urwid.WidgetWrap, mywid.Searchable):
key += '-' + str(comment.line)
key += '-' + path
comment_list = comment_lists.get(key, [])
if comment.draft:
message = comment.message
else:
message = [('comment-name', comment.author.name),
('comment', u': '+comment.message)]
comment_list.append((comment.key, message))
#if comment.draft:
# message = comment.message
#else:
# message = [('comment-name', comment.author.name),
# ('comment', u': '+comment.message)]
comment_list.append((comment.key, comment))
comment_lists[key] = comment_list
comment_filenames.add(path)
repo = gitrepo.get_repo(self.project_name, self.app.config)

View File

@ -70,17 +70,52 @@ class SideDiffComment(BaseDiffComment):
def __init__(self, context, old, new):
super(SideDiffComment, self).__init__([])
self.context = context
oldt = urwid.Text(old)
newt = urwid.Text(new)
if old:
oldt = urwid.AttrMap(oldt, 'comment')
if new:
newt = urwid.AttrMap(newt, 'comment')
oldt = self.formatText(old)
newt = self.formatText(new)
self.contents.append((urwid.Text(u''), ('given', LN_COL_WIDTH, False)))
self.contents.append((oldt, ('weight', 1, False)))
self.contents.append((urwid.Text(u''), ('given', LN_COL_WIDTH, False)))
self.contents.append((newt, ('weight', 1, False)))
def x(self, button):
pass
def formatText(self, comment):
if not comment:
return urwid.Text('')
if comment.draft:
message = comment.message
else:
message = [('comment-name', comment.author.name),
('comment', u': '+comment.message)]
text = urwid.Text(message)
ret = urwid.AttrMap(text, 'comment')
if comment._thread.comments[-1] is comment:
# Last comment in a thread
buttons = [
mywid.FixedButton(('comment-thread-button', "Reply"),
on_press=self.x),
mywid.FixedButton(('comment-thread-button', "Quote"),
on_press=self.x),
]
if comment._thread.unresolved:
buttons.append(
mywid.FixedButton(('comment-thread-button', "Ack"),
on_press=self.x))
buttons.append(
mywid.FixedButton(('comment-thread-button', "Done"),
on_press=self.x))
buttons = [('pack', urwid.AttrMap(b, None, focus_map=self.focus_map)) for b in buttons]
buttons = urwid.Columns(buttons + [urwid.Text('')], dividechars=2)
buttons = urwid.AttrMap(buttons, 'comment-thread-button')
pile = urwid.Pile([ret, buttons])
ret = pile
return ret
class SideDiffLine(BaseDiffLine):
def __init__(self, app, context, old, new, callback=None):
super(SideDiffLine, self).__init__('', on_press=callback)
@ -157,8 +192,12 @@ class SideDiffView(BaseDiffView):
old_comment = new_comment = u''
if old_list:
(old_comment_key, old_comment) = old_list.pop(0)
if not old_list:
pass #XXX last comment
if new_list:
(new_comment_key, new_comment) = new_list.pop(0)
if not new_list:
pass #XXX last comment
lines.append(SideDiffComment(context, old_comment, new_comment))
# see if there are any draft comments for this line
key = 'olddraft-%s-%s' % (old[0], diff.oldname)
@ -170,8 +209,10 @@ class SideDiffView(BaseDiffView):
old_comment = new_comment = u''
if old_list:
(old_comment_key, old_comment) = old_list.pop(0)
old_comment = old_comment.message
if new_list:
(new_comment_key, new_comment) = new_list.pop(0)
new_comment = new_comment.message
lines.append(SideDiffCommentEdit(self.app, context,
old_comment_key,
new_comment_key,
@ -197,8 +238,12 @@ class SideDiffView(BaseDiffView):
old_comment = new_comment = u''
if old_list:
(old_comment_key, old_comment) = old_list.pop(0)
if not old_list:
pass #XXX last comment
if new_list:
(new_comment_key, new_comment) = new_list.pop(0)
if not new_list:
pass #XXX last comment
lines.append(SideDiffComment(context, old_comment, new_comment))
# see if there are any draft comments for this file
key = 'olddraft-None-%s' % (diff.oldname,)
@ -210,8 +255,10 @@ class SideDiffView(BaseDiffView):
old_comment = new_comment = u''
if old_list:
(old_comment_key, old_comment) = old_list.pop(0)
old_comment = old_comment.message
if new_list:
(new_comment_key, new_comment) = new_list.pop(0)
new_comment = new_comment.message
lines.append(SideDiffCommentEdit(self.app, context,
old_comment_key,
new_comment_key,