Commit 63bcff079120449a73ea2cceec00484d3378041c
Start adding line comments
cel committed on 11/21/2017, 9:40:11 PMParent: 47dad06f4a54a8840baed0e2b5ad39a6ba75cf16
Files changed
index.js | changed |
lib/forms.js | changed |
lib/repos/index.js | changed |
lib/repos/pulls.js | changed |
locale/en.json | changed |
locale/eo.json | changed |
schemas.md | added |
index.js | ||
---|---|---|
@@ -327,8 +327,46 @@ | ||
327 | 327 … | if (err) return cb(null, self.serveError(req, err)) |
328 | 328 … | cb(null, self.serveRedirect(req, req.url)) |
329 | 329 … | }) |
330 | 330 … | |
331 … | + case 'line-comment': | |
332 … | + if (!data.repo) | |
333 … | + return cb(null, self.serveError(req, | |
334 … | + new ParamError('missing repo id'), 400)) | |
335 … | + if (!data.commitId) | |
336 … | + return cb(null, self.serveError(req, | |
337 … | + new ParamError('missing commit id'), 400)) | |
338 … | + if (!data.filePath) | |
339 … | + return cb(null, self.serveError(req, | |
340 … | + new ParamError('missing file path'), 400)) | |
341 … | + if (!data.line) | |
342 … | + return cb(null, self.serveError(req, | |
343 … | + new ParamError('missing line number'), 400)) | |
344 … | + var lineNumber = Number(data.line) | |
345 … | + if (isNaN(lineNumber)) | |
346 … | + return cb(null, self.serveError(req, | |
347 … | + new ParamError('bad line number'), 400)) | |
348 … | + var repoBranches = data.repoBranch | |
349 … | + ? data.repoBranch.split(',') : '' | |
350 … | + var msg = { | |
351 … | + type: 'line-comment', | |
352 … | + text: data.text, | |
353 … | + repo: data.repo, | |
354 … | + repoBranch: repoBranches, | |
355 … | + commitId: data.commitId, | |
356 … | + filePath: data.filePath, | |
357 … | + line: lineNumber, | |
358 … | + } | |
359 … | + msg.issue = data.issue | |
360 … | + var mentions = Mentions(data.text) | |
361 … | + if (mentions.length) | |
362 … | + msg.mentions = mentions | |
363 … | + return cb(null, self.serveBuffer(200, JSON.stringify(msg, 0, 2))) | |
364 … | + return self.ssb.publish(msg, function (err) { | |
365 … | + if (err) return cb(null, self.serveError(req, err)) | |
366 … | + cb(null, self.serveRedirect(req, req.url)) | |
367 … | + }) | |
368 … | + | |
331 | 369 … | case 'new-issue': |
332 | 370 … | var msg = Issues.schemas.new(dir, data.text) |
333 | 371 … | var mentions = Mentions(data.text) |
334 | 372 … | if (mentions.length) |
lib/forms.js | ||
---|---|---|
@@ -10,9 +10,11 @@ | ||
10 | 10 … | req._t('post.Write') + '</label>' + |
11 | 11 … | '<label for="tab2" id="preview-tab-link" class="tab2-link">' + |
12 | 12 … | req._t('post.Preview') + '</label>' + |
13 | 13 … | '</div>' + |
14 | - '<input type="hidden" id="repo-id" value="' + repo.id + '"/>' + | |
14 … | + (repo ? | |
15 … | + '<input type="hidden" id="repo-id" value="' + repo.id + '"/>' | |
16 … | + : '') + | |
15 | 17 … | '<div id="write-tab" class="tab1">' + |
16 | 18 … | '<textarea id="post-text" name="text" class="wide-input"' + |
17 | 19 … | ' rows="' + (rows||4) + '" cols="77"' + |
18 | 20 … | (placeholder ? ' placeholder="' + placeholder + '"' : '') + |
@@ -99,4 +101,18 @@ | ||
99 | 101 … | '/>' + |
100 | 102 … | '<script>' + issueCommentButtonScript + '</script>' + |
101 | 103 … | '</form></section>' |
102 | 104 … | } |
105 … | + | |
106 … | +forms.lineComment = function (req, repo, repoBranch, commitId, filePath, line) { | |
107 … | + return '<section><form action="" method="post">' + | |
108 … | + '<input type="hidden" name="action" value="line-comment">' + | |
109 … | + '<input type="hidden" name="repoBranch" value="' + repoBranch.join(',') + '">' + | |
110 … | + '<input type="hidden" name="repo" value="' + repo.id + '">' + | |
111 … | + '<input type="hidden" name="commitId" value="' + commitId + '">' + | |
112 … | + '<input type="hidden" name="filePath" value="' + filePath + '">' + | |
113 … | + '<input type="hidden" name="line" value="' + line + '">' + | |
114 … | + forms.post(req, repo) + | |
115 … | + '<input type="submit" class="btn open" value="' + | |
116 … | + req._t('issue.LineComment') + '" />' + | |
117 … | + '</form></section>' | |
118 … | +} |
lib/repos/index.js | ||
---|---|---|
@@ -593,8 +593,9 @@ | ||
593 | 593 … | /* Repo commit */ |
594 | 594 … | |
595 | 595 … | R.serveRepoCommit = function (req, repo, rev) { |
596 | 596 … | var self = this |
597 … | + var repoBranch = [] | |
597 | 598 … | return u.readNext(function (cb) { |
598 | 599 … | repo.getCommitParsed(rev, function (err, commit) { |
599 | 600 … | if (err) return cb(null, |
600 | 601 … | self.serveRepoTemplate(req, repo, null, rev, `%{author}/%{repo}@${rev}`, |
@@ -627,9 +628,9 @@ | ||
627 | 628 … | }).join('<br>') + |
628 | 629 … | '</section>' + |
629 | 630 … | '<section><h3>' + req._t('FilesChanged') + '</h3>'), |
630 | 631 … | // TODO: show diff from all parents (merge commits) |
631 | - self.renderDiffStat(req, [repo, repo], [commit.parents[0], commit.id]), | |
632 … | + self.renderDiffStat(req, [repo, repo], [commit.parents[0], commit.id], commit.id, repoBranch), | |
632 | 633 … | pull.once('</section>') |
633 | 634 … | ]))) |
634 | 635 … | }) |
635 | 636 … | }) |
@@ -672,9 +673,9 @@ | ||
672 | 673 … | |
673 | 674 … | |
674 | 675 … | /* Diff stat */ |
675 | 676 … | |
676 | -R.renderDiffStat = function (req, repos, treeIds) { | |
677 … | +R.renderDiffStat = function (req, repos, treeIds, commit, repoBranch) { | |
677 | 678 … | if (treeIds.length == 0) treeIds = [null] |
678 | 679 … | var id = treeIds[0] |
679 | 680 … | var lastI = treeIds.length - 1 |
680 | 681 … | var oldTree = treeIds[0] |
@@ -737,18 +738,19 @@ | ||
737 | 738 … | getRepoObjectString(repos[0], item.id[0], mode0, done()) |
738 | 739 … | getRepoObjectString(repos[1], item.id[lastI], modeI, done()) |
739 | 740 … | done(function (err, strOld, strNew) { |
740 | 741 … | if (err) return cb(err) |
741 | - cb(null, htmlLineDiff(req, item.filename, item.filename, | |
742 … | + var repo = repos[1] | |
743 … | + cb(null, htmlLineDiff(req, repo, repoBranch, commit, item.filename, item.filename, | |
742 | 744 … | strOld, strNew, |
743 | 745 … | u.encodeLink(item.blobPath), !isSubmodule)) |
744 | 746 … | }) |
745 | 747 … | }, 4) |
746 | 748 … | ) |
747 | 749 … | ]) |
748 | 750 … | } |
749 | 751 … | |
750 | -function htmlLineDiff(req, filename, anchor, oldStr, newStr, blobHref, | |
752 … | +function htmlLineDiff(req, repo, repoBranch, commit, filename, anchor, oldStr, newStr, blobHref, | |
751 | 753 … | showViewLink) { |
752 | 754 … | return '<pre><table class="code">' + |
753 | 755 … | '<tr><th colspan=3 id="' + u.escape(anchor) + '">' + filename + |
754 | 756 … | (showViewLink === false ? '' : |
@@ -756,16 +758,17 @@ | ||
756 | 758 … | '<a href="' + blobHref + '">' + req._t('View') + '</a> ' + |
757 | 759 … | '</span>') + |
758 | 760 … | '</th></tr>' + |
759 | 761 … | (oldStr.length + newStr.length > 200000 |
760 | - ? '<tr><td class="diff-info">' + req._t('diff.TooLarge') + '<br>' + | |
762 … | + ? '<tr><td class="diff-info" colspan=3>' + req._t('diff.TooLarge') + '<br>' + | |
761 | 763 … | req._t('diff.OldFileSize', {bytes: oldStr.length}) + '<br>' + |
762 | 764 … | req._t('diff.NewFileSize', {bytes: newStr.length}) + '</td></tr>' |
763 | - : tableDiff(oldStr, newStr, filename)) + | |
765 … | + : tableDiff(req, repo, repoBranch, commit, oldStr, newStr, filename)) + | |
764 | 766 … | '</table></pre>' |
765 | 767 … | } |
766 | 768 … | |
767 | -function tableDiff(oldStr, newStr, filename) { | |
769 … | +function tableDiff(req, repo, repoBranch, commit, oldStr, newStr, filename) { | |
770 … | + var query = req._u.query | |
768 | 771 … | var diff = JsDiff.structuredPatch('', '', oldStr, newStr) |
769 | 772 … | var groups = diff.hunks.map(function (hunk) { |
770 | 773 … | var oldLine = hunk.oldStart |
771 | 774 … | var newLine = hunk.newStart |
@@ -780,14 +783,25 @@ | ||
780 | 783 … | var trClass = s == '+' ? 'diff-new' : s == '-' ? 'diff-old' : '' |
781 | 784 … | var lineNums = [s == '+' ? '' : oldLine++, s == '-' ? '' : newLine++] |
782 | 785 … | var id = [filename].concat(lineNums).join('-') |
783 | 786 … | return '<tr id="' + u.escape(id) + '" class="' + trClass + '">' + |
784 | - lineNums.map(function (num) { | |
787 … | + lineNums.map(function (num, i) { | |
788 … | + var idEnc = encodeURIComponent(id) | |
785 | 789 … | return '<td class="code-linenum">' + |
786 | - (num ? '<a href="#' + encodeURIComponent(id) + '">' + | |
787 | - num + '</a>' : '') + '</td>' | |
790 … | + (num ? '<a href="#' + idEnc + '">' + | |
791 … | + num + '</a>' + | |
792 … | + (i === lineNums.length-1 && s !== '-' ? | |
793 … | + // TODO: use a more descriptive icon for the comment action | |
794 … | + ' <a href="?comment=' + idEnc + '#' + idEnc + '">…</a>' | |
795 … | + : '') | |
796 … | + : '') + '</td>' | |
788 | 797 … | }).join('') + |
789 | - '<td class="code-text">' + html + '</td></tr>' | |
798 … | + '<td class="code-text">' + html + '</td></tr>' + | |
799 … | + (commit && query.comment === id ? | |
800 … | + '<tr><td colspan=4>' + | |
801 … | + forms.lineComment(req, repo, repoBranch, commit, filename, lineNums[lineNums.length-1]) + | |
802 … | + '</td></tr>' | |
803 … | + : '') | |
790 | 804 … | })) |
791 | 805 … | }) |
792 | 806 … | return [].concat.apply([], groups).join('') |
793 | 807 … | } |
lib/repos/pulls.js | ||
---|---|---|
@@ -112,13 +112,14 @@ | ||
112 | 112 … | if (!revs.head) return cb(null, pull.once('<section>' + |
113 | 113 … | req._t('pullRequest.NoChanges') + '</section>')) |
114 | 114 … | GitRepo.getMergeBase(baseRepo, revs.base, headRepo, revs.head, |
115 | 115 … | function (err, mergeBase) { |
116 … | + var repoBranch = [] | |
116 | 117 … | if (err) return cb(err) |
117 | 118 … | cb(null, cat([ |
118 | 119 … | pull.once('<section>'), |
119 | 120 … | self.repo.renderDiffStat(req, |
120 | - [baseRepo, headRepo], [mergeBase, revs.head]), | |
121 … | + [baseRepo, headRepo], [mergeBase, revs.head], revs.head, repoBranch), | |
121 | 122 … | pull.once('</section>') |
122 | 123 … | ])) |
123 | 124 … | } |
124 | 125 … | ) |
locale/en.json | ||
---|---|---|
@@ -130,8 +130,9 @@ | ||
130 | 130 … | "Mentioned": "%{name} mentioned this %{type}", |
131 | 131 … | "MentionedIn": "%{name} mentioned this %{type} in %{post}", |
132 | 132 … | "Renamed": "%{author} renamed this %{type} to %{name}", |
133 | 133 … | "Comment": "Comment", |
134 … | + "LineComment": "Line Comment", | |
134 | 135 … | "Close": "Close %{type}", |
135 | 136 … | "CommentAndClose": "Comment and Close", |
136 | 137 … | "Reopen": "Reopen %{type}", |
137 | 138 … | "CommentAndReopen": "Comment and Reopen" |
locale/eo.json | ||
---|---|---|
@@ -130,8 +130,9 @@ | ||
130 | 130 … | "Mentioned": "%{name} menciis tiun %{type}", |
131 | 131 … | "MentionedIn": "%{name} menciis tiun %{type} en %{post}", |
132 | 132 … | "Renamed": "%{author} renomiĝis tiun %{type} al %{name}", |
133 | 133 … | "Comment": "Komenti", |
134 … | + "LineComment": "Linio-Komenti", | |
134 | 135 … | "Close": "Fermi %{type}", |
135 | 136 … | "CommentAndClose": "Komenti kaj Fermi", |
136 | 137 … | "Reopen": "Remalfermi %{type}", |
137 | 138 … | "CommandAndReopen": "Komenti kaj Remalfermi" |
schemas.md | ||
---|---|---|
@@ -1,0 +1,19 @@ | ||
1 … | +### Line comment | |
2 … | + | |
3 … | +``` | |
4 … | +{ | |
5 … | + "type": "line-comment", | |
6 … | + "text": string, | |
7 … | + "repo": MsgId, | |
8 … | + "repoBranch": MgsIds, | |
9 … | + "line": number, | |
10 … | +} | |
11 … | +``` | |
12 … | +`repo`: id of `git-repo` | |
13 … | +`repoBranch`: id(s) of `git-update` messages that pushed the git commit, git trees, and git blobs needed to render the diff. | |
14 … | +`commitId`: commit that the comment is on | |
15 … | +`filePath`: path to the file that the comment is on | |
16 … | +`line`: line number of the file that the comment is on | |
17 … | + | |
18 … | +Replies to a line comment should be messages of type `post` with `root` | |
19 … | +linking to the `line-comment` message. |
Built with git-ssb-web