Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make linenumbers relative to file #71

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions code_comments/comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ def validate(self):

def href(self):
if self.is_comment_to_file:
href = self.req.href.browser(self.path, rev=self.revision,
href = self.req.href.browser(self.reponame + '/' + self.path, rev=self.revision,
codecomment=self.id)
elif self.is_comment_to_changeset:
href = self.req.href.changeset(self.revision, codecomment=self.id)
href = self.req.href.changeset(self.revision + '/' + self.reponame, codecomment=self.id)
elif self.is_comment_to_attachment:
href = self.req.href('/attachment/ticket/%d/%s'
% (self.attachment_ticket,
Expand Down
10 changes: 9 additions & 1 deletion code_comments/comments.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,23 @@ class Comments(object):

def __init__(self, req, env):
self.req, self.env = req, env
self.valid_sorting_methods = ('id', 'author', 'time', 'path', 'text')
self.valid_sorting_methods = ('id', 'author', 'time', 'reponame', 'path', 'text')

def comment_from_row(self, row):
return Comment(self.req, self.env, row)

def get_filter_values(self):
comments = self.all()
return {
'repos': self.get_all_repos(comments),
'paths': self.get_all_paths(comments),
'authors': self.get_all_comment_authors(comments),
}

def get_all_repos(self, comments):
# Skip the empty string which is the repository for comments on attachments
return sorted(list(set([comment.reponame for comment in comments if comment.reponame != ''])))

def get_all_paths(self, comments):
def get_directory(path):
parts = os.path.split(path)[0].split('/')
Expand Down Expand Up @@ -96,6 +101,9 @@ def get_condition_str_and_corresponding_values(self, args):
elif name.endswith('__lt'):
name = name.replace('__lt', '')
conditions.append(name + ' < %s')
elif name.endswith('__ne'):
name = name.replace('__ne', '')
conditions.append(name + ' != %s')
elif name.endswith('__prefix'):
values.append(
args[name].replace('%', '\\%').replace('_', '\\_') + '%')
Expand Down
58 changes: 57 additions & 1 deletion code_comments/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
from trac.db.schema import Table, Column, Index
from trac.env import IEnvironmentSetupParticipant
from trac.db.api import DatabaseManager
from trac.versioncontrol.api import RepositoryManager

# Database version identifier for upgrades.
db_version = 3
db_version = 5
db_version_key = 'code_comments_schema_version'

# Database schema
Expand All @@ -21,8 +22,11 @@
Column('author'),
Column('time', type='int'),
Column('type'),
Column('reponame'),
Index(['path']),
Index(['author']),
Index(['reponame', 'path']),
Index(['revision', 'reponame']),
],
'code_comments_subscriptions': Table('code_comments_subscriptions',
key=('id', 'user', 'type', 'path',
Expand Down Expand Up @@ -76,9 +80,61 @@ def upgrade_from_2_to_3(env):
dbm.create_tables((schema['code_comments_subscriptions'],))


def upgrade_from_3_to_4(env):
with env.db_transaction as db:
# Add the new column "reponame" and indexes.
db('ALTER TABLE code_comments ADD COLUMN reponame text')
db('CREATE INDEX code_comments_reponame_path_idx ON code_comments (reponame, path)')
db('CREATE INDEX code_comments_revision_reponame_idx ON code_comments (revision, reponame)')

# Comments on attachments need to have the empty string as the reponame instead of NULL.
db("UPDATE code_comments SET reponame = '' WHERE type = 'attachment'")

# Comments on changesets have the reponame in the 'path' column.
db("UPDATE code_comments SET reponame = path, path = '' WHERE type = 'changeset'")

# Comments on files have the reponame as the first component of the 'path' column.
db("""
UPDATE code_comments
SET
reponame = substr(path, 1, instr(path, '/') - 1),
path = substr(path, instr(path, '/') + 1)
WHERE
type = 'browser'
""")


def upgrade_from_4_to_5(env):
with env.db_transaction as db:
# The line numbers of all present comments on changesets are bogus,
# see https://github.com/trac-hacks/trac-code-comments-plugin/issues/67
# We therefore set them to 0 detaching the comment from the line. We leave a note in
# the text of the comment explaining this.

notice = '\n\nThis comment was created by a previous version of the '\
"code-comments plugin and '''is not properly attached to a line of code'''. "\
'See [https://github.com/trac-hacks/trac-code-comments-plugin/issues/67 '\
'issue #67].\n\nThe comment was originally placed on line $oldLineNumber$ of '\
'the diff as it was displayed when the comment was created.'
notice = notice.replace("'", "''")
sql = """
UPDATE code_comments
SET
line = 0,
text = text || REPLACE('{0}', '$oldLineNumber$', line)
WHERE
type = 'changeset'
AND
line != 0
"""
db(sql.format(notice))


upgrade_map = {
2: upgrade_from_1_to_2,
3: upgrade_from_2_to_3,
4: upgrade_from_3_to_4,
5: upgrade_from_4_to_5
}


Expand Down
65 changes: 50 additions & 15 deletions code_comments/htdocs/code-comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ var underscore = _.noConflict();
},
defaultFetchParams: {
path: CodeComments.path || undefined,
reponame: CodeComments.reponame,
revision: CodeComments.revision,
type: CodeComments.page,
},
fetchPageComments: function() {
return this.fetch( { data: _.extend( { line: 0 }, this.defaultFetchParams ) } );
},
fetchLineComments: function() {
return this.fetch( { data: _.extend( { line__gt: 0 }, this.defaultFetchParams ) } );
return this.fetch( { data: _.extend( { line__ne: 0 }, this.defaultFetchParams ) } );
}
});

Expand Down Expand Up @@ -111,13 +112,15 @@ var underscore = _.noConflict();
},
addOne: function(comment) {
var line = comment.get('line');
if (!this.viewPerLine[line]) {
this.viewPerLine[line] = new CommentsForALineView( { line: line } );
var file = comment.get('path');
var key = 'file_' + file + ':' + line;
if (!this.viewPerLine[key]) {
this.viewPerLine[key] = new CommentsForALineView( { file: file, line: line } );

var $tr = $( Rows.getTrByLineNumber( line ) );
$tr.after(this.viewPerLine[line].render().el).addClass('with-comments');
var $tr = $( Rows.getTrByFileAndLineNumberInFile( file, line ) );
$tr.after(this.viewPerLine[key].render().el).addClass('with-comments');
}
this.viewPerLine[line].addOne(comment);
this.viewPerLine[key].addOne(comment);
},
addAll: function() {
var view = this;
Expand All @@ -133,6 +136,7 @@ var underscore = _.noConflict();
template: _.template(CodeComments.templates.comments_for_a_line),
initialize: function(attrs) {
this.line = attrs.line;
this.file = attrs.file;
},
events: {
'click button': 'showAddCommentDialog'
Expand Down Expand Up @@ -217,6 +221,7 @@ var underscore = _.noConflict();
text: text,
author: CodeComments.username,
path: this.path,
reponame: CodeComments.reponame,
revision: CodeComments.revision,
line: line,
type: CodeComments.page
Expand All @@ -237,7 +242,7 @@ var underscore = _.noConflict();
var callbackMouseover = function( event ) {
var row = new RowView( { el: this } ),
file = row.getFile(),
line = row.getLineNumber(),
line = row.getLineNumberInFile(),
displayLine = row.getDisplayLine();
row.replaceLineNumberCellContent( '<a title="Comment on this line" href="#L' + line + '" class="bubble"><span class="ui-icon ui-icon-comment"></span></a>' );

Expand Down Expand Up @@ -266,11 +271,33 @@ var underscore = _.noConflict();
// wrap TH content in spans so we can hide/show them
this.wrapTHsInSpans();
},
getLineByTR: function( tr ) {
return $.inArray( tr, this.$rows ) + 1;
},
getTrByLineNumber: function( line ) {
return this.$rows[line - 1];
getTrByFileAndLineNumberInFile: function ( file, line) {
var col;
var container;
if (CodeComments.page == "browser" && CodeComments.path == file) {
container = $( 'thead', 'table.code' );
col = 0;
} else {
if (line < 0) {
line = -line;
col = 0;
} else {
col = 1
}
var containers = $( 'thead', 'table.code, table.trac-diff' );
for ( var i = 0; (container = containers[i]) != null; i++ ) {
if ($(container).parents( 'li' ).find( 'h2>a:first' ).text() == file) {
break;
}
}
}
var trs = $(container).parents( 'table' ).find( 'tbody>tr' ).not('.comments');
for ( var i = 0, tr; (tr = trs[i]) != null; i++ ) {
if ($(tr).find( 'th>span' )[col].textContent == line) {
return tr;
}
}
return null;
},
wrapTHsInSpans: function() {
$( 'th', this.$rows ).each( function( i, elem ) {
Expand Down Expand Up @@ -301,11 +328,19 @@ var underscore = _.noConflict();
getFile: function() {
return this.$el.parents( 'li' ).find( 'h2>a:first' ).text();
},
getLineNumber: function() {
return Rows.getLineByTR( this.el );
getLineNumberInFile: function() {
var lineNumber = this.$lineNumberCell.text().trim();
if (lineNumber)
return lineNumber;
else
return this.$th.first().text().trim() * -1;
},
getDisplayLine: function() {
return this.$lineNumberCell.text().trim() || this.$th.first().text() + ' (deleted)';
var lineNumber = this.getLineNumberInFile()
if (lineNumber > 0)
return lineNumber;
else
return lineNumber * -1 + ' (deleted)';
}
} );

Expand Down
14 changes: 6 additions & 8 deletions code_comments/subscription.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,13 @@ def from_comment(cls, env, comment, user=None, notify=True):

# Munge changesets and browser
if comment.type in ('changeset', 'browser'):
rm = RepositoryManager(env)
reponame, repos, path = rm.get_repository_by_path(comment.path)
if comment.type == 'browser':
sub['path'] = path
sub['path'] = comment.path
else:
sub['path'] = ''
sub['repos'] = reponame or '(default)'
sub['repos'] = comment.reponame
rm = RepositoryManager(env)
repos = rm.get_repository(comment.reponame)
try:
_cs = repos.get_changeset(comment.revision)
except NoSuchChangeset:
Expand Down Expand Up @@ -296,11 +296,9 @@ def for_comment(cls, env, comment, notify=None):
args['rev'] = str(comment.revision)

if comment.type == 'browser':
rm = RepositoryManager(env)
reponame, _, path = rm.get_repository_by_path(comment.path)
args['type'] = ('browser', 'changeset')
args['path'] = (path, '')
args['repos'] = reponame
args['path'] = (comment.path, '')
args['repos'] = comment.reponame
args['rev'] = (str(comment.revision), '')

return cls.select(env, args, notify)
Expand Down
7 changes: 7 additions & 0 deletions code_comments/templates/comments.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ <h1>Code Comments</h1>
<button id="send-to-ticket" type="submit" data-url="${href('code-comments', 'create-ticket')}">Create ticket with selected</button>
&nbsp;Filter comments:
<form action="${href('code-comments')}" method="GET" style="display: inline;">
<select id="filter-by-repo" name="filter-by-repo">
<option value='' selected="selected">-- All repositories --</option>
<option py:for="repo in repos"
selected="${current_repo_selection == repo or None}"
value="$repo" py:content="repo"></option>
</select>
<select id="filter-by-path" name="filter-by-path">
<option value='' selected="selected">-- All paths --</option>
<option py:for="path in paths"
Expand Down Expand Up @@ -49,6 +55,7 @@ <h1>Code Comments</h1>
<td>$comment.id</td>
<th>$comment.author</th>
<th>${comment.formatted_date()}</th>
<td>$comment.reponame</td>
<td>${comment.path_link_tag()}</td>
<td>$comment.html</td>
<td>${comment.get_ticket_links()}</td>
Expand Down
17 changes: 13 additions & 4 deletions code_comments/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,23 +121,26 @@ def templates_js_data(self):
def changeset_js_data(self, req, data):
return {
'page': 'changeset',
'reponame': data['reponame'],
'revision': data['new_rev'],
'path': data['reponame'],
'path': '',
'selectorToInsertAfter': 'div.diff div.diff:last'
}

def browser_js_data(self, req, data):
return {
'page': 'browser',
'reponame': data['reponame'],
'revision': data['rev'],
'path': (data['reponame'] or '') + '/' + data['path'],
'path': data['path'],
'selectorToInsertAfter': 'table.code'
}

def attachment_js_data(self, req, data):
path = req.path_info.replace('/attachment/', 'attachment:/')
return {
'page': 'attachment',
'reponame': '',
'revision': 0,
'path': path,
'selectorToInsertAfter': 'div#preview'
Expand Down Expand Up @@ -195,9 +198,15 @@ def post_process_request(self, req, template, data, content_type):
return template, data, content_type

def add_path_and_author_filters(self):
self.data['current_repo_selection'] = ''
self.data['current_path_selection'] = ''
self.data['current_author_selection'] = ''

if self.req.args.get('filter-by-repo'):
self.args['reponame'] = \
self.req.args['filter-by-repo']
self.data['current_repo_selection'] = \
self.req.args['filter-by-repo']
if self.req.args.get('filter-by-path'):
self.args['path__prefix'] = \
self.req.args['filter-by-path']
Expand Down Expand Up @@ -240,9 +249,9 @@ def href_with_page(page):

def prepare_sortable_headers(self):
displayed_sorting_methods = \
('id', 'author', 'time', 'path', 'text')
('id', 'author', 'time', 'reponame', 'path', 'text')
displayed_sorting_method_names = \
('ID', 'Author', 'Date', 'Path', 'Text')
('ID', 'Author', 'Date', 'Repository', 'Path', 'Text')
query_args = self.req.args
if 'page' in query_args:
del query_args['page']
Expand Down