https://www.mediawiki.org/wiki/Special:Code/MediaWiki/108350
Revision: 108350
Author: johnduhart
Date: 2012-01-08 06:55:19 +0000 (Sun, 08 Jan 2012)
Log Message:
-----------
Reverting inline commenting from CodeReview
This change is still very buggy and has not recieved any love since initial
commit. From the beginning it was an expertimental feature.
Instead of letting this sit in trunk forever, I'm just going to revert it. If
you'd like to bring it back, finish it completely or make a branch.
ping r95429, r95435, r95680, r95680, r96173, r99030, r99034, r107068, r107069,
r107074
Modified Paths:
--------------
trunk/extensions/CodeReview/CodeReview.php
trunk/extensions/CodeReview/api/ApiCodeDiff.php
trunk/extensions/CodeReview/api/ApiQueryCodeComments.php
trunk/extensions/CodeReview/api/ApiRevisionUpdate.php
trunk/extensions/CodeReview/backend/CodeComment.php
trunk/extensions/CodeReview/backend/CodeRevision.php
trunk/extensions/CodeReview/backend/DiffHighlighter.php
trunk/extensions/CodeReview/codereview.sql
trunk/extensions/CodeReview/modules/ext.codereview.styles.css
trunk/extensions/CodeReview/tests/CodeReviewApiTest.php
trunk/extensions/CodeReview/ui/CodeCommentsListView.php
trunk/extensions/CodeReview/ui/CodeRevisionCommitter.php
trunk/extensions/CodeReview/ui/CodeRevisionView.php
Removed Paths:
-------------
trunk/extensions/CodeReview/archives/code_comment_patch_line.sql
trunk/extensions/CodeReview/modules/ext.codereview.linecomment.js
Modified: trunk/extensions/CodeReview/CodeReview.php
===================================================================
--- trunk/extensions/CodeReview/CodeReview.php 2012-01-08 06:44:32 UTC (rev
108349)
+++ trunk/extensions/CodeReview/CodeReview.php 2012-01-08 06:55:19 UTC (rev
108350)
@@ -167,10 +167,6 @@
'scripts' => 'ext.codereview.loaddiff.js'
) + $commonModuleInfo;
-$wgResourceModules['ext.codereview.linecomment'] = array(
- 'scripts' => 'ext.codereview.linecomment.js'
-) + $commonModuleInfo;
-
// Revision tooltips CodeRevisionView:
$wgResourceModules['ext.codereview.tooltips'] = array(
'scripts' => 'ext.codereview.tooltips.js',
@@ -354,8 +350,6 @@
$updater->addExtensionIndex( 'code_rev',
'cr_repo_status_author',
"$base/archives/code_revs_status_author-index.sql" );
- $updater->addExtensionField( 'code_comment', 'cc_patch_line',
- "$base/archives/code_comment_patch_line.sql" );
$updater->addExtensionUpdate( array( 'dropField',
'code_comment', 'cc_review',
"$base/archives/code_drop_cc_review.sql", true ) );
@@ -371,8 +365,6 @@
"$base/archives/code_signoffs_timestamp_struck.sql",
true ) );
$updater->addExtensionUpdate( array( 'addIndex', 'code_paths',
'repo_path',
"$base/archives/codereview-repopath.sql", true ) );
- $updater->addExtensionUpdate( array( 'addField',
'code_comment', 'cc_patch_line',
- "$base/archives/code_comment_patch_line.sql", true ) );
break;
case 'postgres':
// TODO
Modified: trunk/extensions/CodeReview/api/ApiCodeDiff.php
===================================================================
--- trunk/extensions/CodeReview/api/ApiCodeDiff.php 2012-01-08 06:44:32 UTC
(rev 108349)
+++ trunk/extensions/CodeReview/api/ApiCodeDiff.php 2012-01-08 06:55:19 UTC
(rev 108350)
@@ -47,7 +47,6 @@
$html = 'Diff too large.';
} else {
$hilite = new CodeDiffHighlighter();
- # Fetch diff rendered without inline comments
$html = $hilite->render( $diff );
}
Modified: trunk/extensions/CodeReview/api/ApiQueryCodeComments.php
===================================================================
--- trunk/extensions/CodeReview/api/ApiQueryCodeComments.php 2012-01-08
06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/api/ApiQueryCodeComments.php 2012-01-08
06:55:19 UTC (rev 108350)
@@ -95,9 +95,6 @@
if ( isset( $this->props['text'] ) ) {
ApiResult::setContent( $item, $row->cc_text );
}
- if ( isset( $this->props['patchline'] ) ) {
- $item['patchline'] = $row->cc_patch_line;
- }
return $item;
}
@@ -125,7 +122,6 @@
'user',
'status',
'text',
- 'patchline',
'revid',
'revision',
),
Modified: trunk/extensions/CodeReview/api/ApiRevisionUpdate.php
===================================================================
--- trunk/extensions/CodeReview/api/ApiRevisionUpdate.php 2012-01-08
06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/api/ApiRevisionUpdate.php 2012-01-08
06:55:19 UTC (rev 108350)
@@ -66,14 +66,12 @@
$params['removeflags'],
$params['addreferences'],
$params['removereferences'],
- $params['comment'],
- null, // parent
- 0, // review
- $params['patchline']
+ $params['comment']
);
// Forge a response object
$r = array( 'result' => 'Success' );
+
if ( $commentID !== 0 ) {
// id inserted
$r['commentid'] = intval($commentID);
@@ -81,7 +79,6 @@
$view = new CodeRevisionView( $repo, $rev);
$comment = CodeComment::newFromID( $commentID, $rev );
$r['HTML'] = $view->formatComment( $comment );
- //$r['HTML'] = print_r( $comment, true );
}
$this->getResult()->addValue( null, $this->getModuleName(), $r
);
@@ -137,10 +134,6 @@
ApiBase::PARAM_TYPE => 'integer',
ApiBase::PARAM_ISMULTI => true,
),
- 'patchline' => array(
- ApiBase::PARAM_TYPE => 'integer',
- ApiBase::PARAM_MIN => 1,
- ),
);
}
@@ -156,7 +149,6 @@
'removeflags' => 'Code Signoff flags to strike from the
revision by the current user',
'addreferences' => 'Add references to this revision',
'removereferences' => 'Remove references from this
revision',
- 'patchline' => 'Diff line to attach the comment to
(optional)',
);
}
Deleted: trunk/extensions/CodeReview/archives/code_comment_patch_line.sql
===================================================================
--- trunk/extensions/CodeReview/archives/code_comment_patch_line.sql
2012-01-08 06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/archives/code_comment_patch_line.sql
2012-01-08 06:55:19 UTC (rev 108350)
@@ -1,2 +0,0 @@
-ALTER TABLE /*_*/code_comment
- ADD COLUMN cc_patch_line int default null;
Modified: trunk/extensions/CodeReview/backend/CodeComment.php
===================================================================
--- trunk/extensions/CodeReview/backend/CodeComment.php 2012-01-08 06:44:32 UTC
(rev 108349)
+++ trunk/extensions/CodeReview/backend/CodeComment.php 2012-01-08 06:55:19 UTC
(rev 108350)
@@ -4,7 +4,7 @@
* Represents a comment made to a revision.
*/
class CodeComment {
- public $id, $text, $user, $userText, $timestamp, $sortkey, $attrib,
$removed, $added, $patchLine;
+ public $id, $text, $user, $userText, $timestamp, $sortkey, $attrib,
$removed, $added;
/**
* @var CodeRevision
@@ -68,7 +68,6 @@
$comment->user = $data['cc_user'];
$comment->userText = $data['cc_user_text'];
$comment->timestamp = wfTimestamp( TS_MW, $data['cc_timestamp']
);
- $comment->patchLine = $data['cc_patch_line'];
$comment->sortkey = $data['cc_sortkey'];
return $comment;
}
Modified: trunk/extensions/CodeReview/backend/CodeRevision.php
===================================================================
--- trunk/extensions/CodeReview/backend/CodeRevision.php 2012-01-08
06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/backend/CodeRevision.php 2012-01-08
06:55:19 UTC (rev 108350)
@@ -656,16 +656,15 @@
* @param $text
* @param $review
* @param null $parent
- * @param int $patchLine (default: null)
* @return int
*/
- public function saveComment( $text, $review, $parent = null, $patchLine
= null ) {
+ public function saveComment( $text, $review, $parent = null ) {
$text = rtrim( $text );
if ( !strlen( $text ) ) {
return 0;
}
$dbw = wfGetDB( DB_MASTER );
- $data = $this->commentData( $text, $review, $parent, $patchLine
);
+ $data = $this->commentData( $text, $review, $parent );
$dbw->begin();
$data['cc_id'] = $dbw->nextSequenceValue( 'code_comment_cc_id'
);
@@ -735,10 +734,9 @@
* @param $text
* @param $review
* @param null $parent
- * @param int $patchLine (default: null)
* @return array
*/
- protected function commentData( $text, $review, $parent = null,
$patchLine = null ) {
+ protected function commentData( $text, $review, $parent = null ) {
global $wgUser;
$dbw = wfGetDB( DB_MASTER );
$ts = wfTimestamp( TS_MW );
@@ -748,7 +746,6 @@
'cc_rev_id' => $this->id,
'cc_text' => $text,
'cc_parent' => $parent,
- 'cc_patch_line' => $patchLine,
'cc_user' => $wgUser->getId(),
'cc_user_text' => $wgUser->getName(),
'cc_timestamp' => $dbw->timestamp( $ts ),
@@ -782,21 +779,9 @@
}
/**
- * @param $attached boolean Fetch comment attached to a line of code
(default: false)
* @return array
*/
- public function getComments( $attached = false ) {
- $conditions = array(
- 'cc_repo_id' => $this->repoId,
- 'cc_rev_id' => $this->id
- );
-
- if( $attached ) {
- $conditions[] = 'cc_patch_line != null';
- } else {
- $conditions['cc_patch_line'] = null;
- }
-
+ public function getComments() {
$dbr = wfGetDB( DB_SLAVE );
$result = $dbr->select( 'code_comment',
array(
@@ -804,10 +789,11 @@
'cc_text',
'cc_user',
'cc_user_text',
- 'cc_patch_line',
'cc_timestamp',
'cc_sortkey' ),
- $conditions,
+ array(
+ 'cc_repo_id' => $this->repoId,
+ 'cc_rev_id' => $this->id ),
__METHOD__,
array(
'ORDER BY' => 'cc_sortkey' )
Modified: trunk/extensions/CodeReview/backend/DiffHighlighter.php
===================================================================
--- trunk/extensions/CodeReview/backend/DiffHighlighter.php 2012-01-08
06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/backend/DiffHighlighter.php 2012-01-08
06:55:19 UTC (rev 108350)
@@ -14,67 +14,19 @@
protected $lineNumber = 0;
/**
- * @var CodeRepositor The repository for this revision
- */
- protected $repo = null;
-
- /**
- * @var CodeRevision revision the diff comes from
- */
- protected $rev = null;
-
- /**
- * Comments inside the diff.
- * initialized with fetchInlineComments()
- */
- private $inlineComments = null;
-
- /**
* Main entry point. Given a diff text, highlight it
* and wrap it in a div
- * Pass both $repos and $rev to have the diff rendered with inline
comments
*
* @param $text string Text to highlight
- * @param $repo CodeRepository (default: null)
- * @param $repo CodeRevision (default: null)
* @return string
*/
- function render( $text, CodeRepository $repo = null, CodeRevision $rev
= null ) {
- if( $repo xor $rev ) {
- throw new MWException( __METHOD__ . " must have both
repository and revision or none of them\n" );
- } elseif( $repo && $rev ) {
- $this->repo = $repo;
- $this->rev = $rev;
- $this->fetchInlineComments();
- }
-
+ function render( $text ) {
return '<table class="mw-codereview-diff">' .
$this->splitLines( $text ) .
"</table>\n";
}
/**
- * Fetch comments attached to a patch line.
- * Comments can be accessed through the array inlineComments. Its
format is:
- * array(
- * line# => array(
- * CodeComment, CodeComment ...
- * ),
- * line# => array(
- * CodeComment, CodeComment ...
- * ),
- * ...
- * );
- */
- private function fetchInlineComments() {
- $inline = $this->rev->getComments( 'which are attached' );
- foreach( $inline as $comment ) {
- $line = $comment->patchLine; # absolute line number in
the diff
- $this->inlineComments[$line][] = $comment;
- }
- }
-
- /**
* Given a bunch of text, split it into individual
* lines, color them, then put it back into one big
* string
@@ -128,39 +80,14 @@
$r = $this->handleLineFile( $line );
}
- # Finally add up any lineComments that might apply to this line
- $r .= $this->addLineComments();
-
# Return HTML generated by one of the handler
return $r;
}
- function addLineComments() {
- $return = '';
- if( !isset( $this->inlineComments ) ) {
- # No inline comments for this revision.
- return $return;
- }
- if( !array_key_exists( $this->lineNumber, $this->inlineComments
) ) {
- # Line does not have any comment applying to
- return $return;
- }
-
- $comments = $this->inlineComments[$this->lineNumber];
- # FIXME: comment formatting can only be handled by a view
- # Would need abstraction, for example as a class of views
helpers.
- $view = new CodeRevisionView( $this->repo, $this->rev );
- foreach( $comments as $comment ) {
- $return .= "<li>{$view->formatComment( $comment
)}</li>\n" ;
- }
- return "<tr class=\"mw-codereview-inlineComment\"><td
colspan=\"3\"><ul>{$return}</ul></td></tr>\n";
- }
-
function formatLine( $content, $class = null ) {
if( is_null($class) ) {
- return Html::rawElement( 'tr',
- array_merge( $this->getLineIdAttr(), array(
'class' => 'commentable' ) ),
+ return Html::rawElement( 'tr', $this->getLineIdAttr(),
Html::Element( 'td', array(
'class'=>'linenumbers' ), $this->left )
. Html::Element( 'td', array(
'class'=>'linenumbers' ), $this->right )
. Html::Element( 'td', array() ,
$content )
@@ -190,8 +117,7 @@
}
$classAttr = is_null($class) ? array() : array( 'class' =>
$class );
- return Html::rawElement( 'tr',
- array_merge( $this->getLineIdAttr(), array( 'class' =>
'commentable' ) ),
+ return Html::rawElement( 'tr', $this->getLineIdAttr(),
Html::rawElement( 'td', array(
'class'=>'linenumbers' ), $left )
. Html::rawElement( 'td', array(
'class'=>'linenumbers' ), $right )
. Html::Element( 'td', $classAttr, $content )
Modified: trunk/extensions/CodeReview/codereview.sql
===================================================================
--- trunk/extensions/CodeReview/codereview.sql 2012-01-08 06:44:32 UTC (rev
108349)
+++ trunk/extensions/CodeReview/codereview.sql 2012-01-08 06:55:19 UTC (rev
108350)
@@ -173,9 +173,6 @@
-- cc_id of parent comment if a threaded child, otherwise NULL
cc_parent int,
- -- patch line the comment eventually applies to or NULL
- cc_patch_line int default null,
-
-- User id/name of the commenter
cc_user int not null,
cc_user_text varchar(255) not null,
Deleted: trunk/extensions/CodeReview/modules/ext.codereview.linecomment.js
===================================================================
--- trunk/extensions/CodeReview/modules/ext.codereview.linecomment.js
2012-01-08 06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/modules/ext.codereview.linecomment.js
2012-01-08 06:55:19 UTC (rev 108350)
@@ -1,110 +0,0 @@
-( function( $ ) {
-var $rev = 0;
-
-window.CodeReview = $.extend( window.CodeReview, {
-
- /* initialization from PHP */
- lcInit: function( rev ) {
- $rev = rev;
- // Register click() event to each diff lines
- $( 'table.mw-codereview-diff tr.commentable').click(
- function () { CodeReview.lcShowForm( $(this) ); }
- );
- },
-
- /**
- * Show the line comment code below a line of code
- * @param lineCode jQuery object
- */
- lcShowForm: function( lineCode ) {
- // Make sure the line id is an integer:
- var lineNumber = parseInt( lineCode.attr('id') ) + 0;
- // Forge the line comment HTML id:
- var htmlId = 'comment-for-' + lineNumber;
-
- lineCode.unbind( 'click' );
- lineCode.click( function () {
- $( '#'+htmlId ).fadeOut( 200 ).remove();
-
- lineCode.unbind( 'click' );
- lineCode.click( function() {
- CodeReview.lcShowForm( lineCode );
- });
- });
-
- var lineComment =
- $('<tr id="' + htmlId + '"><td colspan="3">'
- +'<textarea id="lineCommentContent" rows="5"></textarea><br/>'
- +'<input id="lineCommentSend" type="button" value="Send">'
- +'</td></tr>');
- lineComment.insertAfter( lineCode );
- $( '#lineCommentContent' ).focus();
-
- $( '#lineCommentSend' ).click( function() {
- CodeReview.lcSubmitComment(
- lineCode,
- lineComment,
- $( '#lineCommentContent' ).val()
- );
- });
- },
-
- lcSubmitComment: function( lineCode, lineComment, comment ) {
-
- // retrieve line number from the code line id attribute:
- var lineId = lineCode.attr('id');
- lineId = lineId.substring( lineId.indexOf( '-' ) + 1);
-
- $.ajax({
- url: mw.util.wikiScript( 'api' ),
- data: {
- 'action': 'coderevisionupdate',
-
- 'repo' : mw.config.get(
'wgCodeReviewRepository' ),
- 'rev' : $rev,
-
- 'comment': comment,
- 'patchline' : lineId,
-
- 'format': 'json',
- },
- dataType: 'json',
- type: 'POST',
- success: function( data ) {
- // our API return usage error as a success!
- if( data.error !== undefined ) {
- console.log( lineComment.find( 'input'
) );
- lineComment.find( 'input' ).after(
- $('<span
class="errorbox"></span>')
- .text( data.error.info )
- );
- return;
- }
-
- var text = data.coderevisionupdate.HTML
- lineComment.fadeOut( 200 ).remove();
-
- // check we have a list to insert the comment
in.
- var next = lineCode.next( '.inlineComment' );
- if( next.length === 0 ) {
- $( '<tr class="inlineComment"><td
colspan="3"><ul><li>'+text+'</li></ul></td></tr>' ).insertAfter( lineCode )
- } else {
- lineCode.next('.inlineComment').find(
'ul' ).append(
- $( '<li>' + text + '</li>' )
- );
- }
-
-
- // rebind event handler
- lineCode.click(
- function () { CodeReview.lcShowForm(
$(this) ); }
- );
- },
- error: function() {
- console.log( "AJAX ERROR" );
- }
- });
- },
-
-}); // window.CodeReview
-})( jQuery );
Modified: trunk/extensions/CodeReview/modules/ext.codereview.styles.css
===================================================================
--- trunk/extensions/CodeReview/modules/ext.codereview.styles.css
2012-01-08 06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/modules/ext.codereview.styles.css
2012-01-08 06:55:19 UTC (rev 108350)
@@ -58,15 +58,6 @@
padding: 16px 16px 16px 48px;
}
-/** list containers to insert comments inside the diff table */
-.mw-codereview-inlineComment ul {
- list-style-image: none; /* override Vector default */
- list-style-type: none;
- padding:0;
- padding-left: 2em;
- margin-bottom: 8px;
-}
-
.mw-codereview-status-new,
.mw-codereview-status-new td {
background: #ffffc0 !important;
Modified: trunk/extensions/CodeReview/tests/CodeReviewApiTest.php
===================================================================
--- trunk/extensions/CodeReview/tests/CodeReviewApiTest.php 2012-01-08
06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/tests/CodeReviewApiTest.php 2012-01-08
06:55:19 UTC (rev 108350)
@@ -68,7 +68,6 @@
$data = $this->doApiRequest( array(
'action' => 'coderevisionupdate',
'rev' => 777,
- 'patchline' => 51,
'comment' => 'Awesome comment',
) + $this->commonApiData );
Modified: trunk/extensions/CodeReview/ui/CodeCommentsListView.php
===================================================================
--- trunk/extensions/CodeReview/ui/CodeCommentsListView.php 2012-01-08
06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/ui/CodeCommentsListView.php 2012-01-08
06:55:19 UTC (rev 108350)
@@ -57,8 +57,6 @@
'cr_status' => wfMsg( 'code-field-status' ),
'cr_message' => wfMsg( 'code-field-message' ),
'cc_text' => wfMsg( 'code-field-text' ),
- # patch line is only used for API call. No need for an
i18n message
- 'cc_patch_line' => null,
);
}
Modified: trunk/extensions/CodeReview/ui/CodeRevisionCommitter.php
===================================================================
--- trunk/extensions/CodeReview/ui/CodeRevisionCommitter.php 2012-01-08
06:44:32 UTC (rev 108349)
+++ trunk/extensions/CodeReview/ui/CodeRevisionCommitter.php 2012-01-08
06:55:19 UTC (rev 108350)
@@ -58,12 +58,11 @@
* @param string $commentText Comment to add to the revision
* @param null|int $parent What the parent comment is (if a subcomment)
* @param int $review (unused)
- * @param int $patchLine Patch line number to which the comment will be
attached (default: null).
* @return int Comment ID if added, else 0
*/
public function revisionUpdate( $status, $addTags, $removeTags,
$addSignoffs, $strikeSignoffs,
$addReferences,
$removeReferences, $commentText,
- $parent = null, $review = 0,
$patchLine = null) {
+ $parent = null, $review = 0 ) {
if ( !$this->mRev ) {
return false;
}
@@ -111,7 +110,7 @@
$commentId = 0;
if ( strlen( $commentText ) && $this->validPost(
'codereview-post-comment' ) ) {
// $isPreview = $wgRequest->getCheck( 'wpPreview' );
- $commentId = $this->mRev->saveComment( $commentText,
$review, $parent, $patchLine );
+ $commentId = $this->mRev->saveComment( $commentText,
$review, $parent );
$commentAdded = ($commentId !== 0);
}
Modified: trunk/extensions/CodeReview/ui/CodeRevisionView.php
===================================================================
--- trunk/extensions/CodeReview/ui/CodeRevisionView.php 2012-01-08 06:44:32 UTC
(rev 108349)
+++ trunk/extensions/CodeReview/ui/CodeRevisionView.php 2012-01-08 06:55:19 UTC
(rev 108350)
@@ -202,16 +202,6 @@
}
$html .= xml::closeElement( 'form' );
- // Encode revision id for our modules
- $encRev = Xml::encodeJsVar( $this->mRev->getId() );
-
- if( $wgCodeReviewInlineComments ) {
- $wgOut->addModules( 'ext.codereview.linecomment' );
- $wgOut->addInlineScript(
- "$(document).ready( CodeReview.lcInit( $encRev
) );"
- );
- }
-
$wgOut->addHTML( $html );
}
@@ -470,8 +460,7 @@
return htmlspecialchars( wfMsg(
'code-rev-diff-too-large' ) );
} else {
$hilite = new CodeDiffHighlighter();
- # Diff rendered with inline comments
- return $hilite->render( $diff, $this->mRepo,
$this->mRev );
+ return $hilite->render( $diff );
}
}
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs