diff options
Diffstat (limited to 'extensions/Splinter')
| -rw-r--r-- | extensions/Splinter/lib/Util.pm | 22 | ||||
| -rw-r--r-- | extensions/Splinter/template/en/default/pages/splinter.html.tmpl | 13 | ||||
| -rw-r--r-- | extensions/Splinter/template/en/default/pages/splinter/help.html.tmpl | 54 | ||||
| -rw-r--r-- | extensions/Splinter/web/splinter.css | 13 | ||||
| -rw-r--r-- | extensions/Splinter/web/splinter.js | 97 |
5 files changed, 145 insertions, 54 deletions
diff --git a/extensions/Splinter/lib/Util.pm b/extensions/Splinter/lib/Util.pm index c8c0d52d2..1861d7ab6 100644 --- a/extensions/Splinter/lib/Util.pm +++ b/extensions/Splinter/lib/Util.pm @@ -21,8 +21,6 @@ package Bugzilla::Extension::Splinter::Util; -use strict; - use Bugzilla; use Bugzilla::Util; @@ -68,9 +66,9 @@ sub get_review_url { my $bug_id = $bug->id; if (defined $absolute && $absolute) { - my $urlbase = correct_urlbase(); - $urlbase =~ s!/$!! if $base =~ "^/"; - $base = $urlbase . $base; + my $urlbase = correct_urlbase(); + $urlbase =~ s!/$!! if $base =~ "^/"; + $base = $urlbase . $base; } if ($base =~ /\?/) { @@ -95,12 +93,12 @@ sub munge_create_attachment { my ($bug, $intro_text, $attach_id, $view_link) = @_; if (attachment_id_is_patch ($attach_id)) { - return ("$intro_text" . + return ("$intro_text" . " View: $view_link\015\012" . " Review: " . get_review_url($bug, $attach_id, 1) . "\015\012"); } else { - return ("$intro_text --> ($view_link)"); + return ("$intro_text --> ($view_link)"); } } @@ -117,24 +115,24 @@ sub add_review_links_to_email { if ($email->header('Subject') =~ /^\[Bug\s+(\d+)\]/ && Bugzilla->user->can_see_bug($1)) { - $bug = Bugzilla::Bug->new($1); + $bug = Bugzilla::Bug->new($1); } return unless defined $bug; if ($body =~ /Review\s+of\s+attachment\s+\d+\s*:/) { - $body =~ s~(Review\s+of\s+attachment\s+(\d+)\s*:) + $body =~ s~(Review\s+of\s+attachment\s+(\d+)\s*:) ~"$1\015\012 --> (" . get_review_url($bug, $2, 1) . ")" ~egx; - $new_body = 1; + $new_body = 1; } if ($body =~ /Created attachment \d+\015\012 --> /) { - $body =~ s~(Created\ attachment\ (\d+)\015\012) + $body =~ s~(Created\ attachment\ (\d+)\015\012) \ -->\ \(([^\015\012]*)\)[^\015\012]* ~munge_create_attachment($bug, $1, $2, $3) ~egx; - $new_body = 1; + $new_body = 1; } $email->body_set($body) if $new_body; diff --git a/extensions/Splinter/template/en/default/pages/splinter.html.tmpl b/extensions/Splinter/template/en/default/pages/splinter.html.tmpl index a05cc87f9..4728967c7 100644 --- a/extensions/Splinter/template/en/default/pages/splinter.html.tmpl +++ b/extensions/Splinter/template/en/default/pages/splinter.html.tmpl @@ -16,7 +16,6 @@ # Red Hat, Inc. All Rights Reserved. # # Contributor(s): Owen Taylor <otaylor@redhat.com> - # David Lawrence <dkl@mozilla.com> #%] [% PROCESS global/header.html.tmpl @@ -27,7 +26,6 @@ "js/yui/assets/skins/sam/datatable.css", "extensions/Splinter/web/splinter.css" ] javascript_urls = [ "js/yui/element/element-min.js", - "js/yui/connection/connection-min.js", "js/yui/container/container-min.js", "js/yui/button/button-min.js", "js/yui/json/json-min.js", @@ -40,7 +38,7 @@ <script type="text/javascript"> Splinter.configBase = '[% urlbase FILTER none %][% Param('splinter_base') FILTER js %]'; - Splinter.configBugUrl = '[% urlbase FILTER none %]'; + Splinter.configBugzillaUrl = '[% urlbase FILTER none %]'; Splinter.configHaveExtension = true; Splinter.configHelp = '[% urlbase FILTER none %]page.cgi?id=splinter/help.html'; Splinter.configNote = ''; @@ -117,13 +115,14 @@ </div> <div id="bugInfo" style="display: none;"> - <b>[% terms.Bug %] <a id="bugLink"><span id="bugId"></span></a>:</b> + <b>Bug <a id="bugLink"><span id="bugId"></span></a>:</b> <span id="bugShortDesc"></span> - <span id="bugReporter"></span> - <span id="bugCreationDate"></span> </div> <div id="attachInfo" style="display:none;"> + <span id="attachObsolete"></span> <b>Attachment <a id="attachLink"><span id="attachId"></span></a>:</b> <span id="attachDesc"></span> - <span id="attachCreator"></span> - @@ -139,7 +138,7 @@ <div id="error" style="display: none;"> </div> <div id="enterBug" style="display: none;"> - [% terms.Bug %] to review: + Bug to review: <input id="enterBugInput" /> <input id="enterBugGo" type="button" value="Go" /> <div id="chooseReview" style="display: none;"> @@ -168,7 +167,7 @@ <li>To comment on a specific lines in the patch, first select the filename from the file navigation links.</li> <li>Then double click the line you want to review and a comment box will appear below the line.</li> <li>When the review is complete and you publish it, the overview comment and all line specific comments with their context, - will be combined together into a single review comment on the [% terms.bug %] report.</li> + will be combined together into a single review comment on the bug report.</li> <li>For more detailed instructions, read the Splinter <a id='helpLink' target='splinterHelp' href="[% urlbase FILTER none %]page.cgi?id=splinter/help.html">help page</a>. </li> @@ -234,7 +233,7 @@ </form> <div id="buttonBox"> <span id="attachmentStatusSpan">Patch Status: - <select id="attachmentStatus"> </select> + <select id="attachmentStatus"> </select> </span> <input id="publishButton" type="button" value="Publish" /> <input id="cancelButton" type="button" value="Cancel" /> diff --git a/extensions/Splinter/template/en/default/pages/splinter/help.html.tmpl b/extensions/Splinter/template/en/default/pages/splinter/help.html.tmpl index bff004c8f..7c797c94d 100644 --- a/extensions/Splinter/template/en/default/pages/splinter/help.html.tmpl +++ b/extensions/Splinter/template/en/default/pages/splinter/help.html.tmpl @@ -25,15 +25,15 @@ <h2>Splinter Patch Review</h2> <p> - Splinter is an add-on for [% terms.Bugzilla %] to allow conveniently + Splinter is an add-on for Bugzilla to allow conveniently reviewing patches that people have attached to - [% terms.Bugzilla %]. <a href="http://fishsoup.net/software/splinter">More + Bugzilla. <a href="http://fishsoup.net/software/splinter">More information about Splinter</a>. </p> <h3>The patch review view</h3> <p> If you get to Splinter by clicking on a link next to an - attachment in [% terms.Bugzilla %], you are presented with the patch + attachment in Bugzilla, you are presented with the patch review view. This view has a number of different pages that can be switched between with the links at the top of the screen. The first page is the Overview page, the other pages correspond to @@ -69,31 +69,31 @@ </p> <ul> <li> - An overall comment. The text area on the first page allows - you to enter your overall thoughts on the [% terms.bug %]. + An overall comment. The text area on the first page allows + you to enter your overall thoughts on the bug. </li> <li> - Detailed comments on changes within the files. To comment on a - line in a patch, double click on it, and a text area will open - beneath that comment. When you are done, click the Save button - to save your comment or the Cancel button to throw your - comment away. You can double-click on a saved comment to start - editing it again and make further changes. + Detailed comments on changes within the files. To comment on a + line in a patch, double click on it, and a text area will open + beneath that comment. When you are done, click the Save button + to save your comment or the Cancel button to throw your + comment away. You can double-click on a saved comment to start + editing it again and make further changes. </li> <li> - A change to the attachment status. (This is specific to - [% terms.Bugzilla %] instances that have attachment status, which is a - non-upstream patch. It's somewhat similar to attachment flags, - which splinter doesn't currently support displaying or - changing.) This allows you to mark a patch as read to commit - or needing additional work. This is done by changing the - drop-down next to the Publish button. + A change to the attachment status. (This is specific to + Bugzilla instances that have attachment status, which is a + non-upstream patch. It's somewhat similar to attachment flags, + which splinter doesn't currently support displaying or + changing.) This allows you to mark a patch as read to commit + or needing additional work. This is done by changing the + drop-down next to the Publish button. </li> </ul> <p> Once you are done writing your review, go back to Overview page and click the "Publish" button to submit it as a comment on the - [% terms.bug %]. The comment will have a link back to the review page so + bug. The comment will have a link back to the review page so that people can see your comments with the full context. </p> <h3>Saved drafts</h3> @@ -122,31 +122,31 @@ <h3>Uploading patches for review</h3> <p> Splinter doesn't really care how patches are provided to - [% terms.Bugzilla %], as long as they are well-formatted patches. If you are + Bugzilla, as long as they are well-formatted patches. If you are using Git for version control, you can either format changes as patches using <a href="http://www.kernel.org/pub/software/scm/git/docs/git-format-patch.html">'git - format-patch</a> and attach them manually to the [% terms.bug %], or you + format-patch</a> and attach them manually to the bug, or you can use <a href="http://fishsoup.net/software/git-bz">git-bz</a>. git-bz is highly recommended; it automates most of the steps - that Splinter can't handle: it files new [% terms.bugs %], attaches updated - attachments to existing [% terms.bugs %], and closes [% terms.bugs %] when you push the + that Splinter can't handle: it files new bugs, attaches updated + attachments to existing bugs, and closes bugs when you push the corresponding git commits to your central repository. </p> -<h3>The [% terms.bug %] review view</h3> +<h3>The bug review view</h3> <p> Splinter also has a view where it shows all patches attached to - the [% terms.bug %] with their status and links to review them. You are + the bug with their status and links to review them. You are taken to this page after publishing a review. You can also get - to this page with the [% terms.bug %] link in the upper-right corner of the + to this page with the bug link in the upper-right corner of the patch review view. </p> <h3>Your reviews</h3> <p> Splinter can also show you a list of all your draft and published reviews. Access this page with the "Your reviews" - link at the bottom of the [% terms.bug %] review view. In-progress drafts + link at the bottom of the bug review view. In-progress drafts are shown in bold. </p> diff --git a/extensions/Splinter/web/splinter.css b/extensions/Splinter/web/splinter.css index a4b4f0b6f..3f2eb84fe 100644 --- a/extensions/Splinter/web/splinter.css +++ b/extensions/Splinter/web/splinter.css @@ -34,6 +34,11 @@ textarea:focus { margin-bottom: 1em; } +#attachObsolete { + font-weight: bold; + color: #c00000; +} + .attachment-draft .attachment-id, .attachment-draft .attachment-desc { font-weight: bold; } @@ -400,3 +405,11 @@ div.review-patch-comment-text { padding-right: 5px; font-family: monospace; } + +.file-review-label { + font-size: 80%; +} + +.file-reviewed-nav { + text-decoration: line-through; +} diff --git a/extensions/Splinter/web/splinter.js b/extensions/Splinter/web/splinter.js index efcac8b6f..87a8b49d5 100644 --- a/extensions/Splinter/web/splinter.js +++ b/extensions/Splinter/web/splinter.js @@ -394,6 +394,7 @@ Splinter.Patch.File.prototype = { this.filename = filename; this.status = status; this.hunks = hunks; + this.fileReviewed = false; var l = 0; var i; @@ -1153,10 +1154,13 @@ Splinter.ReviewStorage.LocalReviewStorage.prototype = { } }, - saveDraft : function(bug, attachment, review) { + saveDraft : function(bug, attachment, review, extraProps) { var propertyName = this._reviewPropertyName(bug, attachment); - - this._updateOrCreateReviewInfo(bug, attachment, { isDraft: true }); + if (!extraProps) { + extraProps = {}; + } + extraProps.isDraft = true; + this._updateOrCreateReviewInfo(bug, attachment, extraProps); localStorage[propertyName] = "" + review; }, @@ -1313,6 +1317,12 @@ Splinter.haveDraft = function () { } } + for (i = 0; i < Splinter.thePatch.files.length; i++) { + if (Splinter.thePatch.files[i].fileReviewed) { + return true; + } + } + if (Splinter.flagChanged == 1) { return true; } @@ -1374,7 +1384,15 @@ Splinter.saveDraft = function () { var draftSaved = false; if (Splinter.haveDraft()) { - Splinter.reviewStorage.saveDraft(Splinter.theBug, Splinter.theAttachment, Splinter.theReview); + var filesReviewed = {}; + for (var i = 0; i < Splinter.thePatch.files.length; i++) { + var file = Splinter.thePatch.files[i]; + if (file.fileReviewed) { + filesReviewed[file.filename] = true; + } + } + Splinter.reviewStorage.saveDraft(Splinter.theBug, Splinter.theAttachment, Splinter.theReview, + { 'filesReviewed' : filesReviewed }); draftSaved = true; } else { Splinter.reviewStorage.deleteDraft(Splinter.theBug, Splinter.theAttachment, Splinter.theReview); @@ -1897,6 +1915,29 @@ Splinter.addPatchFile = function (file) { fileLabelStatus.appendChild(document.createTextNode(statusString)); fileLabelStatus.appendTo(fileLabel); + var fileReviewed = new Element(document.createElement('span')); + Dom.addClass(fileReviewed, 'file-review'); + Dom.setAttribute(fileReviewed, 'title', 'Indicates that a review has been completed for this file. ' + + 'This is for personal tracking purposes only and has no effect ' + + 'on the published review.'); + fileReviewed.appendTo(fileLabel); + + var fileReviewedInput = new Element(document.createElement('input')); + Dom.setAttribute(fileReviewedInput, 'type', 'checkbox'); + Dom.setAttribute(fileReviewedInput, 'id', 'file-review-checkbox-' + encodeURIComponent(file.filename)); + Dom.setAttribute(fileReviewedInput, 'onchange', "Splinter.toggleFileReviewed('" + + encodeURIComponent(file.filename) + "');"); + if (file.fileReviewed) { + Dom.setAttribute(fileReviewedInput, 'checked', 'true'); + } + fileReviewedInput.appendTo(fileReviewed); + + var fileReviewedLabel = new Element(document.createElement('label')); + Dom.addClass(fileReviewedLabel, 'file-review-label') + Dom.setAttribute(fileReviewedLabel, 'for', 'file-review-checkbox-' + encodeURIComponent(file.filename)); + fileReviewedLabel.appendChild(document.createTextNode(' Reviewed')); + fileReviewedLabel.appendTo(fileReviewed); + var lastHunk = file.hunks[file.hunks.length - 1]; var lastLine = Math.max(lastHunk.oldStart + lastHunk.oldCount - 1, lastHunk.newStart + lastHunk.newCount - 1); @@ -2079,7 +2120,7 @@ Splinter.toggleCollapsed = function (filename, display) { var i; for (i = 0; i < Splinter.thePatch.files.length; i++) { var file = Splinter.thePatch.files[i]; - if ((filename && file.filename == filename) || !filename) { + if (!filename || filename == file.filename) { var fileTableContainer = file.div.getElementsByClassName('file-table-container')[0]; var fileCollapseLink = file.div.getElementsByClassName('file-label-collapse')[0]; if (!display) { @@ -2091,6 +2132,31 @@ Splinter.toggleCollapsed = function (filename, display) { } } +Splinter.toggleFileReviewed = function (filename) { + var checkbox = Dom.get('file-review-checkbox-' + filename); + if (checkbox) { + filename = decodeURIComponent(filename); + for (var i = 0; i < Splinter.thePatch.files.length; i++) { + var file = Splinter.thePatch.files[i]; + if (file.filename == filename) { + file.fileReviewed = checkbox.checked; + + Splinter.saveDraft(); + Splinter.queueUpdateHaveDraft(); + + // Strike through file names to show review was completed + var fileNavLink = Dom.get('switch-' + encodeURIComponent(filename)); + if (file.fileReviewed) { + Dom.addClass(fileNavLink, 'file-reviewed-nav'); + } + else { + Dom.removeClass(fileNavLink, 'file-reviewed-nav'); + } + } + } + } +} + Splinter.showPatchFile = function (file) { Splinter.selectNavigationLink(file.filename); Dom.setStyle('overview', 'display', 'none'); @@ -2208,7 +2274,7 @@ Splinter.start = function () { } } - // We load the saved draft or create a new reeview *after* inserting the existing reviews + // We load the saved draft or create a new review *after* inserting the existing reviews // so that the ordering comes out right. if (Splinter.reviewStorage) { @@ -2221,6 +2287,18 @@ Splinter.start = function () { storedReviews[i].attachmentId == Splinter.theAttachment.id) { Dom.get("restoredLastModified").innerHTML = Splinter.Utils.formatDate(new Date(storedReviews[i].modificationTime)); + // Restore file reviewed checkboxes + if (storedReviews[i].filesReviewed) { + for (var j = 0; j < Splinter.thePatch.files.length; j++) { + var file = Splinter.thePatch.files[j]; + if (storedReviews[i].filesReviewed[file.filename]) { + file.fileReviewed = true; + // Strike through file names to show that review was completed + var fileNavLink = Dom.get('switch-' + encodeURIComponent(file.filename)); + Dom.addClass(fileNavLink, 'file-reviewed-nav'); + } + } + } } } } @@ -2434,7 +2512,7 @@ Splinter.init = function () { } Dom.get("bugId").innerHTML = Splinter.theBug.id; - Dom.get("bugLink").setAttribute('href', Splinter.configBugUrl + "show_bug.cgi?id=" + Splinter.theBug.id); + Dom.get("bugLink").setAttribute('href', Splinter.configBugzillaUrl + "show_bug.cgi?id=" + Splinter.theBug.id); Dom.get("bugShortDesc").innerHTML = YAHOO.lang.escapeHTML(Splinter.theBug.shortDesc); Dom.get("bugReporter").appendChild(document.createTextNode(Splinter.theBug.getReporter())); Dom.get("bugCreationDate").innerHTML = Splinter.Utils.formatDate(Splinter.theBug.creationDate); @@ -2457,11 +2535,14 @@ Splinter.init = function () { } else { Dom.get("attachId").innerHTML = Splinter.theAttachment.id; - Dom.get("attachLink").setAttribute('href', Splinter.configBugUrl + "attachment.cgi?id=" + Splinter.theAttachment.id); + Dom.get("attachLink").setAttribute('href', Splinter.configBugzillaUrl + "attachment.cgi?id=" + Splinter.theAttachment.id); Dom.get("attachDesc").innerHTML = YAHOO.lang.escapeHTML(Splinter.theAttachment.description); Dom.get("attachCreator").appendChild(document.createTextNode(Splinter.Bug._formatWho(Splinter.theAttachment.whoName, Splinter.theAttachment.whoEmail))); Dom.get("attachDate").innerHTML = Splinter.Utils.formatDate(Splinter.theAttachment.date); + if (Splinter.theAttachment.isObsolete) { + Dom.get("attachObsolete").innerHTML = 'OBSOLETE'; + } Dom.setStyle('attachInfo', 'display', 'block'); Dom.setStyle('quickHelpShow', 'display', 'block'); |