diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 73e7c19144..7116134d1a 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,10 +9,10 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => 'c69171e6', - 'core.pkg.js' => '73a06a9f', - 'differential.pkg.css' => '8d8360fb', - 'differential.pkg.js' => '0b037a4f', + 'core.pkg.css' => '7ce5a944', + 'core.pkg.js' => '6e5c894f', + 'differential.pkg.css' => '607c84be', + 'differential.pkg.js' => 'a0212a0b', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => 'a98c0bf7', 'maniphest.pkg.css' => '35995d6d', @@ -61,7 +61,7 @@ return array( 'rsrc/css/application/dashboard/dashboard.css' => '5a205b9d', 'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d', 'rsrc/css/application/differential/add-comment.css' => '7e5900d9', - 'rsrc/css/application/differential/changeset-view.css' => 'bde53589', + 'rsrc/css/application/differential/changeset-view.css' => '489b6995', 'rsrc/css/application/differential/core.css' => '7300a73e', 'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b', 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', @@ -113,7 +113,7 @@ return array( 'rsrc/css/application/uiexample/example.css' => 'b4795059', 'rsrc/css/core/core.css' => '1b29ed61', 'rsrc/css/core/remarkup.css' => 'f06cc20e', - 'rsrc/css/core/syntax.css' => '4234f572', + 'rsrc/css/core/syntax.css' => '220b85f9', 'rsrc/css/core/z-index.css' => '99c0f5eb', 'rsrc/css/diviner/diviner-shared.css' => '4bd263b0', 'rsrc/css/font/font-awesome.css' => '3883938a', @@ -169,7 +169,7 @@ return array( 'rsrc/css/phui/phui-pager.css' => 'd022c7ad', 'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8', 'rsrc/css/phui/phui-policy-section-view.css' => '139fdc64', - 'rsrc/css/phui/phui-property-list-view.css' => 'cad62236', + 'rsrc/css/phui/phui-property-list-view.css' => '807b1632', 'rsrc/css/phui/phui-remarkup-preview.css' => '91767007', 'rsrc/css/phui/phui-segment-bar-view.css' => '5166b370', 'rsrc/css/phui/phui-spacing.css' => 'b05cadc3', @@ -376,8 +376,8 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be', 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', - 'rsrc/js/application/diff/DiffChangeset.js' => 'd0a85a85', - 'rsrc/js/application/diff/DiffChangesetList.js' => '04023d82', + 'rsrc/js/application/diff/DiffChangeset.js' => 'a31ffc00', + 'rsrc/js/application/diff/DiffChangesetList.js' => '0f5c016d', 'rsrc/js/application/diff/DiffInline.js' => 'a4a14a94', 'rsrc/js/application/diff/behavior-preview-link.js' => 'f51e9c17', 'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd', @@ -506,7 +506,7 @@ return array( 'rsrc/js/core/behavior-tokenizer.js' => '3b4899b0', 'rsrc/js/core/behavior-tooltip.js' => '73ecc1f8', 'rsrc/js/core/behavior-user-menu.js' => '60cd9241', - 'rsrc/js/core/behavior-watch-anchor.js' => '0e6d261f', + 'rsrc/js/core/behavior-watch-anchor.js' => '3972dadb', 'rsrc/js/core/behavior-workflow.js' => '9623adc1', 'rsrc/js/core/darkconsole/DarkLog.js' => '3b869402', 'rsrc/js/core/darkconsole/DarkMessage.js' => '26cd4b73', @@ -554,7 +554,7 @@ return array( 'conpherence-thread-manager' => 'aec8e38c', 'conpherence-transaction-css' => '3a3f5e7e', 'd3' => '9d068042', - 'differential-changeset-view-css' => 'bde53589', + 'differential-changeset-view-css' => '489b6995', 'differential-core-view-css' => '7300a73e', 'differential-revision-add-comment-css' => '7e5900d9', 'differential-revision-comment-css' => '7dbc8d1d', @@ -655,7 +655,7 @@ return array( 'javelin-behavior-phabricator-tooltips' => '73ecc1f8', 'javelin-behavior-phabricator-transaction-comment-form' => '2bdadf1a', 'javelin-behavior-phabricator-transaction-list' => '9cec214e', - 'javelin-behavior-phabricator-watch-anchor' => '0e6d261f', + 'javelin-behavior-phabricator-watch-anchor' => '3972dadb', 'javelin-behavior-pholio-mock-edit' => '3eed1f2b', 'javelin-behavior-pholio-mock-view' => '5aa1544e', 'javelin-behavior-phui-dropdown-menu' => '5cf0501a', @@ -773,8 +773,8 @@ return array( 'phabricator-darklog' => '3b869402', 'phabricator-darkmessage' => '26cd4b73', 'phabricator-dashboard-css' => '5a205b9d', - 'phabricator-diff-changeset' => 'd0a85a85', - 'phabricator-diff-changeset-list' => '04023d82', + 'phabricator-diff-changeset' => 'a31ffc00', + 'phabricator-diff-changeset-list' => '0f5c016d', 'phabricator-diff-inline' => 'a4a14a94', 'phabricator-drag-and-drop-file-upload' => '4370900d', 'phabricator-draggable-list' => 'c9ad6f70', @@ -865,7 +865,7 @@ return array( 'phui-pager-css' => 'd022c7ad', 'phui-pinboard-view-css' => '1f08f5d8', 'phui-policy-section-view-css' => '139fdc64', - 'phui-property-list-view-css' => 'cad62236', + 'phui-property-list-view-css' => '807b1632', 'phui-remarkup-preview-css' => '91767007', 'phui-segment-bar-view-css' => '5166b370', 'phui-spacing-css' => 'b05cadc3', @@ -900,7 +900,7 @@ return array( 'sprite-login-css' => '18b368a6', 'sprite-tokens-css' => 'f1896dc5', 'syntax-default-css' => '055fc231', - 'syntax-highlighting-css' => '4234f572', + 'syntax-highlighting-css' => '220b85f9', 'tokens-css' => 'ce5a50bd', 'trigger-rule' => '41b7b4f6', 'trigger-rule-control' => '5faf27b9', @@ -944,10 +944,6 @@ return array( '03e8891f' => array( 'javelin-install', ), - '04023d82' => array( - 'javelin-install', - 'phuix-button-view', - ), '04f8a1e3' => array( 'javelin-behavior', 'javelin-stratcom', @@ -999,12 +995,6 @@ return array( '0d2490ce' => array( 'javelin-install', ), - '0e6d261f' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-vector', - ), '0eaa33a9' => array( 'javelin-behavior', 'javelin-dom', @@ -1015,6 +1005,10 @@ return array( 'javelin-workflow', 'phuix-icon-view', ), + '0f5c016d' => array( + 'javelin-install', + 'phuix-button-view', + ), '111bfd2d' => array( 'javelin-install', ), @@ -1075,6 +1069,9 @@ return array( 'javelin-behavior', 'javelin-request', ), + '220b85f9' => array( + 'syntax-default-css', + ), '225bbb98' => array( 'javelin-install', 'javelin-reactor', @@ -1227,6 +1224,12 @@ return array( 'javelin-install', 'javelin-dom', ), + '3972dadb' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-vector', + ), '398fdf13' => array( 'javelin-behavior', 'trigger-rule-editor', @@ -1260,9 +1263,6 @@ return array( 'javelin-behavior', 'javelin-uri', ), - '4234f572' => array( - 'syntax-default-css', - ), '4370900d' => array( 'javelin-install', 'javelin-util', @@ -1303,6 +1303,9 @@ return array( 'javelin-dom', 'phabricator-draggable-list', ), + '489b6995' => array( + 'phui-inline-comment-view-css', + ), '48fe33d0' => array( 'javelin-behavior', 'javelin-dom', @@ -1785,6 +1788,17 @@ return array( 'javelin-workflow', 'phabricator-draggable-list', ), + 'a31ffc00' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-workflow', + 'javelin-router', + 'javelin-behavior-device', + 'javelin-vector', + 'phabricator-diff-inline', + ), 'a4356cde' => array( 'javelin-install', 'javelin-dom', @@ -1960,9 +1974,6 @@ return array( 'phabricator-drag-and-drop-file-upload', 'javelin-workboard-board', ), - 'bde53589' => array( - 'phui-inline-comment-view-css', - ), 'c03f2fb4' => array( 'javelin-install', ), @@ -2034,17 +2045,6 @@ return array( 'javelin-dom', 'javelin-stratcom', ), - 'd0a85a85' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-workflow', - 'javelin-router', - 'javelin-behavior-device', - 'javelin-vector', - 'phabricator-diff-inline', - ), 'd12d214f' => array( 'javelin-install', 'javelin-dom', diff --git a/resources/sql/autopatches/20190924.diffusion.01.permanent.sql b/resources/sql/autopatches/20190924.diffusion.01.permanent.sql new file mode 100644 index 0000000000..4c84f32a52 --- /dev/null +++ b/resources/sql/autopatches/20190924.diffusion.01.permanent.sql @@ -0,0 +1,2 @@ +ALTER TABLE {$NAMESPACE}_repository.repository_refcursor + ADD isPermanent BOOL NOT NULL; diff --git a/resources/sql/autopatches/20190924.diffusion.02.default.sql b/resources/sql/autopatches/20190924.diffusion.02.default.sql new file mode 100644 index 0000000000..2f639a5f84 --- /dev/null +++ b/resources/sql/autopatches/20190924.diffusion.02.default.sql @@ -0,0 +1,2 @@ +UPDATE {$NAMESPACE}_repository.repository_refcursor + SET isPermanent = 1; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f062b577c3..41befc5ce3 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2150,6 +2150,7 @@ phutil_register_library_map(array( 'PhabricatorAlmanacApplication' => 'applications/almanac/application/PhabricatorAlmanacApplication.php', 'PhabricatorAmazonAuthProvider' => 'applications/auth/provider/PhabricatorAmazonAuthProvider.php', 'PhabricatorAmazonSNSFuture' => 'applications/metamta/future/PhabricatorAmazonSNSFuture.php', + 'PhabricatorAnchorTestCase' => 'infrastructure/markup/__tests__/PhabricatorAnchorTestCase.php', 'PhabricatorAnchorView' => 'view/layout/PhabricatorAnchorView.php', 'PhabricatorAphlictManagementDebugWorkflow' => 'applications/aphlict/management/PhabricatorAphlictManagementDebugWorkflow.php', 'PhabricatorAphlictManagementNotifyWorkflow' => 'applications/aphlict/management/PhabricatorAphlictManagementNotifyWorkflow.php', @@ -2267,6 +2268,7 @@ phutil_register_library_map(array( 'PhabricatorAuthChallengeStatusController' => 'applications/auth/controller/mfa/PhabricatorAuthChallengeStatusController.php', 'PhabricatorAuthChallengeUpdate' => 'applications/auth/view/PhabricatorAuthChallengeUpdate.php', 'PhabricatorAuthChangePasswordAction' => 'applications/auth/action/PhabricatorAuthChangePasswordAction.php', + 'PhabricatorAuthChangeUsernameMessageType' => 'applications/auth/message/PhabricatorAuthChangeUsernameMessageType.php', 'PhabricatorAuthConduitAPIMethod' => 'applications/auth/conduit/PhabricatorAuthConduitAPIMethod.php', 'PhabricatorAuthConduitTokenRevoker' => 'applications/auth/revoker/PhabricatorAuthConduitTokenRevoker.php', 'PhabricatorAuthConfirmLinkController' => 'applications/auth/controller/PhabricatorAuthConfirmLinkController.php', @@ -3136,6 +3138,9 @@ phutil_register_library_map(array( 'PhabricatorDividerProfileMenuItem' => 'applications/search/menuitem/PhabricatorDividerProfileMenuItem.php', 'PhabricatorDivinerApplication' => 'applications/diviner/application/PhabricatorDivinerApplication.php', 'PhabricatorDocumentEngine' => 'applications/files/document/PhabricatorDocumentEngine.php', + 'PhabricatorDocumentEngineBlock' => 'applications/files/diff/PhabricatorDocumentEngineBlock.php', + 'PhabricatorDocumentEngineBlockDiff' => 'applications/files/diff/PhabricatorDocumentEngineBlockDiff.php', + 'PhabricatorDocumentEngineBlocks' => 'applications/files/diff/PhabricatorDocumentEngineBlocks.php', 'PhabricatorDocumentRef' => 'applications/files/document/PhabricatorDocumentRef.php', 'PhabricatorDocumentRenderingEngine' => 'applications/files/document/render/PhabricatorDocumentRenderingEngine.php', 'PhabricatorDoorkeeperApplication' => 'applications/doorkeeper/application/PhabricatorDoorkeeperApplication.php', @@ -4872,6 +4877,7 @@ phutil_register_library_map(array( 'PhabricatorSystemRemoveWorkflow' => 'applications/system/management/PhabricatorSystemRemoveWorkflow.php', 'PhabricatorSystemSelectEncodingController' => 'applications/system/controller/PhabricatorSystemSelectEncodingController.php', 'PhabricatorSystemSelectHighlightController' => 'applications/system/controller/PhabricatorSystemSelectHighlightController.php', + 'PhabricatorSystemSelectViewAsController' => 'applications/system/controller/PhabricatorSystemSelectViewAsController.php', 'PhabricatorTOTPAuthFactor' => 'applications/auth/factor/PhabricatorTOTPAuthFactor.php', 'PhabricatorTOTPAuthFactorTestCase' => 'applications/auth/factor/__tests__/PhabricatorTOTPAuthFactorTestCase.php', 'PhabricatorTaskmasterDaemon' => 'infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php', @@ -5594,9 +5600,13 @@ phutil_register_library_map(array( 'PhutilOAuthAuthAdapter' => 'applications/auth/adapter/PhutilOAuthAuthAdapter.php', 'PhutilPHPCodeSnippetContextFreeGrammar' => 'infrastructure/lipsum/code/PhutilPHPCodeSnippetContextFreeGrammar.php', 'PhutilPhabricatorAuthAdapter' => 'applications/auth/adapter/PhutilPhabricatorAuthAdapter.php', + 'PhutilProseDiff' => 'infrastructure/diff/prose/PhutilProseDiff.php', + 'PhutilProseDiffTestCase' => 'infrastructure/diff/prose/__tests__/PhutilProseDiffTestCase.php', + 'PhutilProseDifferenceEngine' => 'infrastructure/diff/prose/PhutilProseDifferenceEngine.php', 'PhutilQsprintfInterface' => 'infrastructure/storage/xsprintf/PhutilQsprintfInterface.php', 'PhutilQueryString' => 'infrastructure/storage/xsprintf/PhutilQueryString.php', 'PhutilRealNameContextFreeGrammar' => 'infrastructure/lipsum/PhutilRealNameContextFreeGrammar.php', + 'PhutilRemarkupAnchorRule' => 'infrastructure/markup/markuprule/PhutilRemarkupAnchorRule.php', 'PhutilRemarkupBlockInterpreter' => 'infrastructure/markup/blockrule/PhutilRemarkupBlockInterpreter.php', 'PhutilRemarkupBlockRule' => 'infrastructure/markup/blockrule/PhutilRemarkupBlockRule.php', 'PhutilRemarkupBlockStorage' => 'infrastructure/markup/PhutilRemarkupBlockStorage.php', @@ -8314,6 +8324,7 @@ phutil_register_library_map(array( 'PhabricatorAlmanacApplication' => 'PhabricatorApplication', 'PhabricatorAmazonAuthProvider' => 'PhabricatorOAuth2AuthProvider', 'PhabricatorAmazonSNSFuture' => 'PhutilAWSFuture', + 'PhabricatorAnchorTestCase' => 'PhabricatorTestCase', 'PhabricatorAnchorView' => 'AphrontView', 'PhabricatorAphlictManagementDebugWorkflow' => 'PhabricatorAphlictManagementWorkflow', 'PhabricatorAphlictManagementNotifyWorkflow' => 'PhabricatorAphlictManagementWorkflow', @@ -8452,6 +8463,7 @@ phutil_register_library_map(array( 'PhabricatorAuthChallengeStatusController' => 'PhabricatorAuthController', 'PhabricatorAuthChallengeUpdate' => 'Phobject', 'PhabricatorAuthChangePasswordAction' => 'PhabricatorSystemAction', + 'PhabricatorAuthChangeUsernameMessageType' => 'PhabricatorAuthMessageType', 'PhabricatorAuthConduitAPIMethod' => 'ConduitAPIMethod', 'PhabricatorAuthConduitTokenRevoker' => 'PhabricatorAuthRevoker', 'PhabricatorAuthConfirmLinkController' => 'PhabricatorAuthController', @@ -9466,6 +9478,9 @@ phutil_register_library_map(array( 'PhabricatorDividerProfileMenuItem' => 'PhabricatorProfileMenuItem', 'PhabricatorDivinerApplication' => 'PhabricatorApplication', 'PhabricatorDocumentEngine' => 'Phobject', + 'PhabricatorDocumentEngineBlock' => 'Phobject', + 'PhabricatorDocumentEngineBlockDiff' => 'Phobject', + 'PhabricatorDocumentEngineBlocks' => 'Phobject', 'PhabricatorDocumentRef' => 'Phobject', 'PhabricatorDocumentRenderingEngine' => 'Phobject', 'PhabricatorDoorkeeperApplication' => 'PhabricatorApplication', @@ -11494,6 +11509,7 @@ phutil_register_library_map(array( 'PhabricatorSystemRemoveWorkflow' => 'PhabricatorManagementWorkflow', 'PhabricatorSystemSelectEncodingController' => 'PhabricatorController', 'PhabricatorSystemSelectHighlightController' => 'PhabricatorController', + 'PhabricatorSystemSelectViewAsController' => 'PhabricatorController', 'PhabricatorTOTPAuthFactor' => 'PhabricatorAuthFactor', 'PhabricatorTOTPAuthFactorTestCase' => 'PhabricatorTestCase', 'PhabricatorTaskmasterDaemon' => 'PhabricatorDaemon', @@ -12387,8 +12403,12 @@ phutil_register_library_map(array( 'PhutilOAuthAuthAdapter' => 'PhutilAuthAdapter', 'PhutilPHPCodeSnippetContextFreeGrammar' => 'PhutilCLikeCodeSnippetContextFreeGrammar', 'PhutilPhabricatorAuthAdapter' => 'PhutilOAuthAuthAdapter', + 'PhutilProseDiff' => 'Phobject', + 'PhutilProseDiffTestCase' => 'PhabricatorTestCase', + 'PhutilProseDifferenceEngine' => 'Phobject', 'PhutilQueryString' => 'Phobject', 'PhutilRealNameContextFreeGrammar' => 'PhutilContextFreeGrammar', + 'PhutilRemarkupAnchorRule' => 'PhutilRemarkupRule', 'PhutilRemarkupBlockInterpreter' => 'Phobject', 'PhutilRemarkupBlockRule' => 'Phobject', 'PhutilRemarkupBlockStorage' => 'Phobject', diff --git a/src/aphront/response/Aphront403Response.php b/src/aphront/response/Aphront403Response.php index 3f7f6d73ca..3d3d5c9457 100644 --- a/src/aphront/response/Aphront403Response.php +++ b/src/aphront/response/Aphront403Response.php @@ -28,7 +28,7 @@ final class Aphront403Response extends AphrontHTMLResponse { $dialog = id(new AphrontDialogView()) ->setUser($user) ->setTitle(pht('403 Forbidden')) - ->addCancelButton('/', pht('Peace Out')) + ->addCancelButton('/', pht('Yikes!')) ->appendParagraph($forbidden_text); $view = id(new PhabricatorStandardPageView()) diff --git a/src/applications/auth/message/PhabricatorAuthChangeUsernameMessageType.php b/src/applications/auth/message/PhabricatorAuthChangeUsernameMessageType.php new file mode 100644 index 0000000000..78b2998f8b --- /dev/null +++ b/src/applications/auth/message/PhabricatorAuthChangeUsernameMessageType.php @@ -0,0 +1,29 @@ +ids = $ids; + return $this; + } + public function withCallerPHIDs(array $phids) { $this->callerPHIDs = $phids; return $this; @@ -41,6 +47,13 @@ final class PhabricatorConduitLogQuery protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { $where = parent::buildWhereClauseParts($conn); + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + if ($this->callerPHIDs !== null) { $where[] = qsprintf( $conn, diff --git a/src/applications/differential/controller/DifferentialChangesetViewController.php b/src/applications/differential/controller/DifferentialChangesetViewController.php index fe6163feb9..f2558ffa1d 100644 --- a/src/applications/differential/controller/DifferentialChangesetViewController.php +++ b/src/applications/differential/controller/DifferentialChangesetViewController.php @@ -166,6 +166,7 @@ final class DifferentialChangesetViewController extends DifferentialController { DifferentialChangesetParser::parseRangeSpecification($spec); $parser = id(new DifferentialChangesetParser()) + ->setViewer($viewer) ->setCoverage($coverage) ->setChangeset($changeset) ->setRenderingReference($rendering_reference) diff --git a/src/applications/differential/parser/DifferentialChangesetParser.php b/src/applications/differential/parser/DifferentialChangesetParser.php index 9843ff3d5d..a485a787cb 100644 --- a/src/applications/differential/parser/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/DifferentialChangesetParser.php @@ -58,6 +58,8 @@ final class DifferentialChangesetParser extends Phobject { private $linesOfContext = 8; private $highlightEngine; + private $viewer; + private $documentEngineKey; public function setRange($start, $end) { $this->rangeStart = $start; @@ -149,6 +151,24 @@ final class DifferentialChangesetParser extends Phobject { return $this->offsetMode; } + public function setViewer(PhabricatorUser $viewer) { + $this->viewer = $viewer; + return $this; + } + + public function getViewer() { + return $this->viewer; + } + + public function setDocumentEngineKey($document_engine_key) { + $this->documentEngineKey = $document_engine_key; + return $this; + } + + public function getDocumentEngineKey() { + return $this->documentEngineKey; + } + public static function getDefaultRendererForViewer(PhabricatorUser $viewer) { $is_unified = $viewer->compareUserSetting( PhabricatorUnifiedDiffsSetting::SETTINGKEY, @@ -164,6 +184,7 @@ final class DifferentialChangesetParser extends Phobject { public function readParametersFromRequest(AphrontRequest $request) { $this->setCharacterEncoding($request->getStr('encoding')); $this->setHighlightAs($request->getStr('highlight')); + $this->setDocumentEngineKey($request->getStr('engine')); $renderer = null; @@ -226,7 +247,7 @@ final class DifferentialChangesetParser extends Phobject { return $this->depthOnlyLines; } - public function setVisibileLinesMask(array $mask) { + public function setVisibleLinesMask(array $mask) { $this->visible = $mask; return $this; } @@ -678,13 +699,13 @@ final class DifferentialChangesetParser extends Phobject { $lines_context = $this->getLinesOfContext(); $hunk_parser->generateIntraLineDiffs(); - $hunk_parser->generateVisibileLinesMask($lines_context); + $hunk_parser->generateVisibleLinesMask($lines_context); $this->setOldLines($hunk_parser->getOldLines()); $this->setNewLines($hunk_parser->getNewLines()); $this->setIntraLineDiffs($hunk_parser->getIntraLineDiffs()); $this->setDepthOnlyLines($hunk_parser->getDepthOnlyLines()); - $this->setVisibileLinesMask($hunk_parser->getVisibleLinesMask()); + $this->setVisibleLinesMask($hunk_parser->getVisibleLinesMask()); $this->hunkStartLines = $hunk_parser->getHunkStartLines( $changeset->getHunks()); @@ -847,8 +868,19 @@ final class DifferentialChangesetParser extends Phobject { ->setHighlightingDisabled($this->highlightingDisabled) ->setDepthOnlyLines($this->getDepthOnlyLines()); + list($engine, $old_ref, $new_ref) = $this->newDocumentEngine(); + if ($engine) { + $engine_blocks = $engine->newEngineBlocks( + $old_ref, + $new_ref); + } else { + $engine_blocks = null; + } + + $has_document_engine = ($engine_blocks !== null); + $shield = null; - if ($this->isTopLevel && !$this->comments) { + if ($this->isTopLevel && !$this->comments && !$has_document_engine) { if ($this->isGenerated()) { $shield = $renderer->renderShield( pht( @@ -1003,79 +1035,39 @@ final class DifferentialChangesetParser extends Phobject { ->setOldComments($old_comments) ->setNewComments($new_comments); + if ($engine_blocks !== null) { + $reference = $this->getRenderingReference(); + $parts = explode('/', $reference); + if (count($parts) == 2) { + list($id, $vs) = $parts; + } else { + $id = $parts[0]; + $vs = 0; + } + + // If we don't have an explicit "vs" changeset, it's the left side of + // the "id" changeset. + if (!$vs) { + $vs = $id; + } + + $renderer + ->setDocumentEngine($engine) + ->setDocumentEngineBlocks($engine_blocks); + + return $renderer->renderDocumentEngineBlocks( + $engine_blocks, + (string)$id, + (string)$vs); + } + + // If we've made it here with a type of file we don't know how to render, + // bail out with a default empty rendering. Normally, we'd expect a + // document engine to catch these changes before we make it this far. switch ($this->changeset->getFileType()) { - case DifferentialChangeType::FILE_IMAGE: - $old = null; - $new = null; - // TODO: Improve the architectural issue as discussed in D955 - // https://secure.phabricator.com/D955 - $reference = $this->getRenderingReference(); - $parts = explode('/', $reference); - if (count($parts) == 2) { - list($id, $vs) = $parts; - } else { - $id = $parts[0]; - $vs = 0; - } - $id = (int)$id; - $vs = (int)$vs; - - if (!$vs) { - $metadata = $this->changeset->getMetadata(); - $data = idx($metadata, 'attachment-data'); - - $old_phid = idx($metadata, 'old:binary-phid'); - $new_phid = idx($metadata, 'new:binary-phid'); - } else { - $vs_changeset = id(new DifferentialChangeset())->load($vs); - $old_phid = null; - $new_phid = null; - - // TODO: This is spooky, see D6851 - if ($vs_changeset) { - $vs_metadata = $vs_changeset->getMetadata(); - $old_phid = idx($vs_metadata, 'new:binary-phid'); - } - - $changeset = id(new DifferentialChangeset())->load($id); - if ($changeset) { - $metadata = $changeset->getMetadata(); - $new_phid = idx($metadata, 'new:binary-phid'); - } - } - - if ($old_phid || $new_phid) { - // grab the files, (micro) optimization for 1 query not 2 - $file_phids = array(); - if ($old_phid) { - $file_phids[] = $old_phid; - } - if ($new_phid) { - $file_phids[] = $new_phid; - } - - $files = id(new PhabricatorFileQuery()) - ->setViewer($this->getUser()) - ->withPHIDs($file_phids) - ->execute(); - foreach ($files as $file) { - if (empty($file)) { - continue; - } - if ($file->getPHID() == $old_phid) { - $old = $file; - } else if ($file->getPHID() == $new_phid) { - $new = $file; - } - } - } - - $renderer->attachOldFile($old); - $renderer->attachNewFile($new); - - return $renderer->renderFileChange($old, $new, $id, $vs); case DifferentialChangeType::FILE_DIRECTORY: case DifferentialChangeType::FILE_BINARY: + case DifferentialChangeType::FILE_IMAGE: $output = $renderer->renderChangesetTable(null); return $output; } @@ -1675,4 +1667,154 @@ final class DifferentialChangesetParser extends Phobject { return $prefix.$line; } + private function newDocumentEngine() { + $changeset = $this->changeset; + $viewer = $this->getViewer(); + + // TODO: This should probably be made non-optional in the future. + if (!$viewer) { + return null; + } + + $old_file = null; + $new_file = null; + + switch ($changeset->getFileType()) { + case DifferentialChangeType::FILE_IMAGE: + case DifferentialChangeType::FILE_BINARY: + list($old_file, $new_file) = $this->loadFileObjectsForChangeset(); + break; + } + + $old_ref = id(new PhabricatorDocumentRef()) + ->setName($changeset->getOldFile()); + if ($old_file) { + $old_ref->setFile($old_file); + } else { + $old_data = $this->old; + $old_data = ipull($old_data, 'text'); + $old_data = implode('', $old_data); + + $old_ref->setData($old_data); + } + + $new_ref = id(new PhabricatorDocumentRef()) + ->setName($changeset->getFilename()); + if ($new_file) { + $new_ref->setFile($new_file); + } else { + $new_data = $this->new; + $new_data = ipull($new_data, 'text'); + $new_data = implode('', $new_data); + + $new_ref->setData($new_data); + } + + $old_engines = PhabricatorDocumentEngine::getEnginesForRef( + $viewer, + $old_ref); + + $new_engines = PhabricatorDocumentEngine::getEnginesForRef( + $viewer, + $new_ref); + + $shared_engines = array_intersect_key($old_engines, $new_engines); + + foreach ($shared_engines as $key => $shared_engine) { + if (!$shared_engine->canDiffDocuments($old_ref, $new_ref)) { + unset($shared_engines[$key]); + } + } + + $engine_key = $this->getDocumentEngineKey(); + if (strlen($engine_key)) { + if (isset($shared_engines[$engine_key])) { + $document_engine = $shared_engines[$engine_key]; + } else { + $document_engine = null; + } + } else { + $document_engine = head($shared_engines); + } + + if ($document_engine) { + return array( + $document_engine, + $old_ref, + $new_ref); + } + + return null; + } + + private function loadFileObjectsForChangeset() { + $changeset = $this->changeset; + $viewer = $this->getViewer(); + + $old_file = null; + $new_file = null; + + // TODO: Improve the architectural issue as discussed in D955 + // https://secure.phabricator.com/D955 + $reference = $this->getRenderingReference(); + $parts = explode('/', $reference); + if (count($parts) == 2) { + list($id, $vs) = $parts; + } else { + $id = $parts[0]; + $vs = 0; + } + $id = (int)$id; + $vs = (int)$vs; + + if (!$vs) { + $metadata = $this->changeset->getMetadata(); + $data = idx($metadata, 'attachment-data'); + + $old_phid = idx($metadata, 'old:binary-phid'); + $new_phid = idx($metadata, 'new:binary-phid'); + } else { + $vs_changeset = id(new DifferentialChangeset())->load($vs); + $old_phid = null; + $new_phid = null; + + // TODO: This is spooky, see D6851 + if ($vs_changeset) { + $vs_metadata = $vs_changeset->getMetadata(); + $old_phid = idx($vs_metadata, 'new:binary-phid'); + } + + $changeset = id(new DifferentialChangeset())->load($id); + if ($changeset) { + $metadata = $changeset->getMetadata(); + $new_phid = idx($metadata, 'new:binary-phid'); + } + } + + if ($old_phid || $new_phid) { + $file_phids = array(); + if ($old_phid) { + $file_phids[] = $old_phid; + } + if ($new_phid) { + $file_phids[] = $new_phid; + } + + $files = id(new PhabricatorFileQuery()) + ->setViewer($viewer) + ->withPHIDs($file_phids) + ->execute(); + + foreach ($files as $file) { + if ($file->getPHID() == $old_phid) { + $old_file = $file; + } else if ($file->getPHID() == $new_phid) { + $new_file = $file; + } + } + } + + return array($old_file, $new_file); + } + } diff --git a/src/applications/differential/parser/DifferentialHunkParser.php b/src/applications/differential/parser/DifferentialHunkParser.php index 84f466adcd..6cdadf69e7 100644 --- a/src/applications/differential/parser/DifferentialHunkParser.php +++ b/src/applications/differential/parser/DifferentialHunkParser.php @@ -36,7 +36,7 @@ final class DifferentialHunkParser extends Phobject { } public function getVisibleLinesMask() { if ($this->visibleLinesMask === null) { - throw new PhutilInvalidStateException('generateVisibileLinesMask'); + throw new PhutilInvalidStateException('generateVisibleLinesMask'); } return $this->visibleLinesMask; } @@ -354,7 +354,7 @@ final class DifferentialHunkParser extends Phobject { return $this; } - public function generateVisibileLinesMask($lines_context) { + public function generateVisibleLinesMask($lines_context) { $old = $this->getOldLines(); $new = $this->getNewLines(); $max_length = max(count($old), count($new)); diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php index df59db9228..c856b9b14d 100644 --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -270,11 +270,20 @@ abstract class DifferentialChangesetHTMLRenderer } } - if ($this->getHighlightingDisabled()) { - $messages[] = pht( - 'This file is larger than %s, so syntax highlighting is '. - 'disabled by default.', - phutil_format_bytes(DifferentialChangesetParser::HIGHLIGHT_BYTE_LIMIT)); + $blocks = $this->getDocumentEngineBlocks(); + if ($blocks) { + foreach ($blocks->getMessages() as $message) { + $messages[] = $message; + } + } else { + if ($this->getHighlightingDisabled()) { + $byte_limit = DifferentialChangesetParser::HIGHLIGHT_BYTE_LIMIT; + $byte_limit = phutil_format_bytes($byte_limit); + $messages[] = pht( + 'This file is larger than %s, so syntax highlighting is '. + 'disabled by default.', + $byte_limit); + } } return $this->formatHeaderMessages($messages); @@ -608,17 +617,4 @@ abstract class DifferentialChangesetHTMLRenderer return array($left_prefix, $right_prefix); } - protected function renderImageStage(PhabricatorFile $file) { - return phutil_tag( - 'div', - array( - 'class' => 'differential-image-stage', - ), - phutil_tag( - 'img', - array( - 'src' => $file->getBestURI(), - ))); - } - } diff --git a/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php index ad8939d275..c149808f12 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpMailRenderer.php @@ -31,14 +31,6 @@ final class DifferentialChangesetOneUpMailRenderer return null; } - public function renderFileChange( - $old_file = null, - $new_file = null, - $id = 0, - $vs = 0) { - return null; - } - public function renderTextChange( $range_start, $range_len, diff --git a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php index 289b802485..fc50b0d605 100644 --- a/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetOneUpRenderer.php @@ -228,60 +228,238 @@ final class DifferentialChangesetOneUpRenderer return null; } - public function renderFileChange( - $old_file = null, - $new_file = null, - $id = 0, - $vs = 0) { + public function renderDocumentEngineBlocks( + PhabricatorDocumentEngineBlocks $block_list, + $old_changeset_key, + $new_changeset_key) { - // TODO: This should eventually merge into the normal primitives pathway, - // but fake it for now and just share as much code as possible. + $engine = $this->getDocumentEngine(); + $layout = $block_list->newTwoUpLayout(); - $primitives = array(); - if ($old_file) { - $primitives[] = array( - 'type' => 'old-file', - 'htype' => ($new_file ? 'new-file' : null), - 'file' => $old_file, - 'line' => 1, - 'render' => $this->renderImageStage($old_file), - ); + $old_comments = $this->getOldComments(); + $new_comments = $this->getNewComments(); + + $unchanged = array(); + foreach ($layout as $key => $row) { + list($old, $new) = $row; + + if (!$old) { + continue; + } + + if (!$new) { + continue; + } + + if ($old->getDifferenceType() !== null) { + continue; + } + + if ($new->getDifferenceType() !== null) { + continue; + } + + $unchanged[$key] = true; } - if ($new_file) { - $primitives[] = array( - 'type' => 'new-file', - 'htype' => ($old_file ? 'old-file' : null), - 'file' => $new_file, - 'line' => 1, - 'oline' => ($old_file ? 1 : null), - 'render' => $this->renderImageStage($new_file), - ); - } + $rows = array(); + $count = count($layout); + for ($ii = 0; $ii < $count;) { + $start = $ii; - // TODO: We'd like to share primitive code here, but buildPrimitives() - // currently chokes on changesets with no textual data. - foreach ($this->getOldComments() as $line => $group) { - foreach ($group as $comment) { - $primitives[] = array( - 'type' => 'inline', - 'comment' => $comment, - 'right' => false, + for ($jj = $ii; $jj < $count; $jj++) { + list($old, $new) = $layout[$jj]; + + if (empty($unchanged[$jj])) { + break; + } + + $rows[] = array( + 'type' => 'unchanged', + 'layoutKey' => $jj, ); } + $ii = $jj; + + for ($jj = $ii; $jj < $count; $jj++) { + list($old, $new) = $layout[$jj]; + + if (!empty($unchanged[$jj])) { + break; + } + + $rows[] = array( + 'type' => 'old', + 'layoutKey' => $jj, + ); + } + + for ($jj = $ii; $jj < $count; $jj++) { + list($old, $new) = $layout[$jj]; + + if (!empty($unchanged[$jj])) { + break; + } + + $rows[] = array( + 'type' => 'new', + 'layoutKey' => $jj, + ); + } + $ii = $jj; + + // We always expect to consume at least one row when iterating through + // the loop and make progress. If we don't, bail out to avoid spinning + // to death. + if ($ii === $start) { + throw new Exception( + pht( + 'Failed to make progress during 1up diff layout.')); + } } - foreach ($this->getNewComments() as $line => $group) { - foreach ($group as $comment) { - $primitives[] = array( - 'type' => 'inline', - 'comment' => $comment, - 'right' => true, - ); + $old_ref = null; + $new_ref = null; + $refs = $block_list->getDocumentRefs(); + if ($refs) { + list($old_ref, $new_ref) = $refs; + } + + $view = array(); + foreach ($rows as $row) { + $row_type = $row['type']; + $layout_key = $row['layoutKey']; + $row_layout = $layout[$layout_key]; + list($old, $new) = $row_layout; + + if ($old) { + $old_key = $old->getBlockKey(); + } else { + $old_key = null; + } + + if ($new) { + $new_key = $new->getBlockKey(); + } else { + $new_key = null; + } + + $cells = array(); + $cell_classes = array(); + + if ($row_type === 'unchanged') { + $cell_content = $engine->newBlockContentView( + $old_ref, + $old); + } else if ($old && $new) { + $block_diff = $engine->newBlockDiffViews( + $old_ref, + $old, + $new_ref, + $new); + + // TODO: We're currently double-rendering this: once when building + // the old row, and once when building the new one. In both cases, + // we throw away the other half of the output. We could cache this + // to improve performance. + + if ($row_type === 'old') { + $cell_content = $block_diff->getOldContent(); + $cell_classes = $block_diff->getOldClasses(); + } else { + $cell_content = $block_diff->getNewContent(); + $cell_classes = $block_diff->getNewClasses(); + } + } else if ($row_type === 'old') { + $cell_content = $engine->newBlockContentView( + $old_ref, + $old); + $cell_classes[] = 'old'; + $cell_classes[] = 'old-full'; + + $new_key = null; + } else if ($row_type === 'new') { + $cell_content = $engine->newBlockContentView( + $new_ref, + $new); + $cell_classes[] = 'new'; + $cell_classes[] = 'new-full'; + + $old_key = null; + } + + if ($old_key === null) { + $old_id = null; + } else { + $old_id = "C{$old_changeset_key}OL{$old_key}"; + } + + if ($new_key === null) { + $new_id = null; + } else { + $new_id = "C{$new_changeset_key}NL{$new_key}"; + } + + $cells[] = phutil_tag( + 'td', + array( + 'id' => $old_id, + 'data-n' => $old_key, + 'class' => 'n', + )); + + $cells[] = phutil_tag( + 'td', + array( + 'id' => $new_id, + 'data-n' => $new_key, + 'class' => 'n', + )); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => 'copy', + )); + + $cell_classes[] = 'diff-flush'; + $cell_classes = implode(' ', $cell_classes); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => $cell_classes, + 'data-copy-mode' => 'copy-unified', + ), + $cell_content); + + $view[] = phutil_tag( + 'tr', + array(), + $cells); + + if ($old_key !== null) { + $old_inlines = idx($old_comments, $old_key, array()); + foreach ($old_inlines as $inline) { + $inline = $this->buildInlineComment( + $inline, + $on_right = false); + $view[] = $this->getRowScaffoldForInline($inline); + } + } + + if ($new_key !== null) { + $new_inlines = idx($new_comments, $new_key, array()); + foreach ($new_inlines as $inline) { + $inline = $this->buildInlineComment( + $inline, + $on_right = true); + $view[] = $this->getRowScaffoldForInline($inline); + } } } - $output = $this->renderPrimitives($primitives, 1); + $output = $this->wrapChangeInTable($view); return $this->renderChangesetTable($output); } diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php index 4ed77bf041..6b1ed8fa27 100644 --- a/src/applications/differential/render/DifferentialChangesetRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetRenderer.php @@ -36,6 +36,9 @@ abstract class DifferentialChangesetRenderer extends Phobject { private $scopeEngine = false; private $depthOnlyLines; + private $documentEngine; + private $documentEngineBlocks; + private $oldFile = false; private $newFile = false; @@ -239,6 +242,25 @@ abstract class DifferentialChangesetRenderer extends Phobject { return $this->oldChangesetID; } + public function setDocumentEngine(PhabricatorDocumentEngine $engine) { + $this->documentEngine = $engine; + return $this; + } + + public function getDocumentEngine() { + return $this->documentEngine; + } + + public function setDocumentEngineBlocks( + PhabricatorDocumentEngineBlocks $blocks) { + $this->documentEngineBlocks = $blocks; + return $this; + } + + public function getDocumentEngineBlocks() { + return $this->documentEngineBlocks; + } + public function setNewComments(array $new_comments) { foreach ($new_comments as $line_number => $comments) { assert_instances_of($comments, 'PhabricatorInlineCommentInterface'); @@ -355,6 +377,16 @@ abstract class DifferentialChangesetRenderer extends Phobject { $notice = null; if ($this->getIsTopLevel()) { $force = (!$content && !$props); + + // If we have DocumentEngine messages about the blocks, assume they + // explain why there's no content. + $blocks = $this->getDocumentEngineBlocks(); + if ($blocks) { + if ($blocks->getMessages()) { + $force = false; + } + } + $notice = $this->renderChangeTypeHeader($force); } @@ -378,11 +410,13 @@ abstract class DifferentialChangesetRenderer extends Phobject { $range_start, $range_len, $rows); - abstract public function renderFileChange( - $old = null, - $new = null, - $id = 0, - $vs = 0); + + public function renderDocumentEngineBlocks( + PhabricatorDocumentEngineBlocks $blocks, + $old_changeset_key, + $new_changeset_key) { + return null; + } abstract protected function renderChangeTypeHeader($force); abstract protected function renderUndershieldHeader(); diff --git a/src/applications/differential/render/DifferentialChangesetTestRenderer.php b/src/applications/differential/render/DifferentialChangesetTestRenderer.php index e2bd3f53ed..56b96dcbca 100644 --- a/src/applications/differential/render/DifferentialChangesetTestRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTestRenderer.php @@ -134,14 +134,4 @@ abstract class DifferentialChangesetTestRenderer return phutil_safe_html($out); } - - public function renderFileChange( - $old_file = null, - $new_file = null, - $id = 0, - $vs = 0) { - - throw new PhutilMethodNotImplementedException(); - } - } diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php index d803e92c6c..a7af9333ff 100644 --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -364,76 +364,220 @@ final class DifferentialChangesetTwoUpRenderer return $this->wrapChangeInTable(phutil_implode_html('', $html)); } - public function renderFileChange( - $old_file = null, - $new_file = null, - $id = 0, - $vs = 0) { + public function renderDocumentEngineBlocks( + PhabricatorDocumentEngineBlocks $block_list, + $old_changeset_key, + $new_changeset_key) { - $old = null; - if ($old_file) { - $old = $this->renderImageStage($old_file); + $engine = $this->getDocumentEngine(); + + $old_ref = null; + $new_ref = null; + $refs = $block_list->getDocumentRefs(); + if ($refs) { + list($old_ref, $new_ref) = $refs; } - $new = null; - if ($new_file) { - $new = $this->renderImageStage($new_file); - } + $old_comments = $this->getOldComments(); + $new_comments = $this->getNewComments(); - // If we don't have an explicit "vs" changeset, it's the left side of the - // "id" changeset. - if (!$vs) { - $vs = $id; - } + $gap_view = javelin_tag( + 'tr', + array( + 'sigil' => 'context-target', + ), + phutil_tag( + 'td', + array( + 'colspan' => 6, + 'class' => 'show-more', + ), + pht("\xE2\x80\xA2 \xE2\x80\xA2 \xE2\x80\xA2"))); - $html_old = array(); - $html_new = array(); - foreach ($this->getOldComments() as $on_line => $comment_group) { - foreach ($comment_group as $comment) { - $inline = $this->buildInlineComment( - $comment, - $on_right = false); - $html_old[] = $this->getRowScaffoldForInline($inline); + $rows = array(); + $in_gap = false; + foreach ($block_list->newTwoUpLayout() as $row) { + list($old, $new) = $row; + + if ($old) { + $old_key = $old->getBlockKey(); + $is_visible = $old->getIsVisible(); + } else { + $old_key = null; } - } - foreach ($this->getNewComments() as $lin_line => $comment_group) { - foreach ($comment_group as $comment) { - $inline = $this->buildInlineComment( - $comment, - $on_right = true); - $html_new[] = $this->getRowScaffoldForInline($inline); + + if ($new) { + $new_key = $new->getBlockKey(); + $is_visible = $new->getIsVisible(); + } else { + $new_key = null; } + + if (!$is_visible) { + if (!$in_gap) { + $in_gap = true; + $rows[] = $gap_view; + } + continue; + } + + if ($in_gap) { + $in_gap = false; + } + + if ($old) { + $is_rem = ($old->getDifferenceType() === '-'); + } else { + $is_rem = false; + } + + if ($new) { + $is_add = ($new->getDifferenceType() === '+'); + } else { + $is_add = false; + } + + if ($is_rem && $is_add) { + $block_diff = $engine->newBlockDiffViews( + $old_ref, + $old, + $new_ref, + $new); + + $old_content = $block_diff->getOldContent(); + $new_content = $block_diff->getNewContent(); + + $old_classes = $block_diff->getOldClasses(); + $new_classes = $block_diff->getNewClasses(); + } else { + $old_classes = array(); + $new_classes = array(); + + if ($old) { + $old_content = $engine->newBlockContentView( + $old_ref, + $old); + + if ($is_rem) { + $old_classes[] = 'old'; + $old_classes[] = 'old-full'; + } + } else { + $old_content = null; + } + + if ($new) { + $new_content = $engine->newBlockContentView( + $new_ref, + $new); + + if ($is_add) { + $new_classes[] = 'new'; + $new_classes[] = 'new-full'; + } + } else { + $new_content = null; + } + } + + $old_classes[] = 'diff-flush'; + $old_classes = implode(' ', $old_classes); + + $new_classes[] = 'diff-flush'; + $new_classes = implode(' ', $new_classes); + + $old_inline_rows = array(); + if ($old_key !== null) { + $old_inlines = idx($old_comments, $old_key, array()); + foreach ($old_inlines as $inline) { + $inline = $this->buildInlineComment( + $inline, + $on_right = false); + $old_inline_rows[] = $this->getRowScaffoldForInline($inline); + } + } + + $new_inline_rows = array(); + if ($new_key !== null) { + $new_inlines = idx($new_comments, $new_key, array()); + foreach ($new_inlines as $inline) { + $inline = $this->buildInlineComment( + $inline, + $on_right = true); + $new_inline_rows[] = $this->getRowScaffoldForInline($inline); + } + } + + if ($old_content === null) { + $old_id = null; + } else { + $old_id = "C{$old_changeset_key}OL{$old_key}"; + } + + $old_line_cell = phutil_tag( + 'td', + array( + 'id' => $old_id, + 'data-n' => $old_key, + 'class' => 'n', + )); + + $old_content_cell = phutil_tag( + 'td', + array( + 'class' => $old_classes, + 'data-copy-mode' => 'copy-l', + ), + $old_content); + + if ($new_content === null) { + $new_id = null; + } else { + $new_id = "C{$new_changeset_key}NL{$new_key}"; + } + + $new_line_cell = phutil_tag( + 'td', + array( + 'id' => $new_id, + 'data-n' => $new_key, + 'class' => 'n', + )); + + $copy_gutter = phutil_tag( + 'td', + array( + 'class' => 'copy', + )); + + $new_content_cell = phutil_tag( + 'td', + array( + 'class' => $new_classes, + 'colspan' => '2', + 'data-copy-mode' => 'copy-r', + ), + $new_content); + + $row_view = phutil_tag( + 'tr', + array(), + array( + $old_line_cell, + $old_content_cell, + $new_line_cell, + $copy_gutter, + $new_content_cell, + )); + + $rows[] = array( + $row_view, + $old_inline_rows, + $new_inline_rows, + ); } - if (!$old) { - $th_old = phutil_tag('th', array()); - } else { - $th_old = phutil_tag('th', array('id' => "C{$vs}OL1"), 1); - } - - if (!$new) { - $th_new = phutil_tag('th', array()); - } else { - $th_new = phutil_tag('th', array('id' => "C{$id}NL1"), 1); - } - - $output = hsprintf( - ''. - '%s'. - '%s'. - '%s'. - '%s'. - ''. - '%s'. - '%s', - $th_old, - $old, - $th_new, - $new, - phutil_implode_html('', $html_old), - phutil_implode_html('', $html_new)); - - $output = $this->wrapChangeInTable($output); + $output = $this->wrapChangeInTable($rows); return $this->renderChangesetTable($output); } diff --git a/src/applications/differential/view/DifferentialChangesetListView.php b/src/applications/differential/view/DifferentialChangesetListView.php index 67568005fa..353457ef04 100644 --- a/src/applications/differential/view/DifferentialChangesetListView.php +++ b/src/applications/differential/view/DifferentialChangesetListView.php @@ -240,6 +240,7 @@ final class DifferentialChangesetListView extends AphrontView { 'View Unified' => pht('View Unified'), 'Change Text Encoding...' => pht('Change Text Encoding...'), 'Highlight As...' => pht('Highlight As...'), + 'View As...' => pht('View As...'), 'Loading...' => pht('Loading...'), diff --git a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php index f9e9f74774..197fa285dc 100644 --- a/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php +++ b/src/applications/diffusion/query/lowlevel/DiffusionLowLevelResolveRefsQuery.php @@ -195,13 +195,15 @@ final class DiffusionLowLevelResolveRefsQuery $alternate = null; if ($type == 'tag') { - $alternate = $identifier; - $identifier = idx($tag_map, $ref); - if (!$identifier) { - throw new Exception( - pht( - "Failed to look up tag '%s'!", - $ref)); + $tag_identifier = idx($tag_map, $ref); + if ($tag_identifier === null) { + // This can happen when we're asked to resolve the hash of a "tag" + // object created with "git tag --annotate" that isn't currently + // reachable from any ref. Just leave things as they are. + } else { + // Otherwise, we have a normal named tag. + $alternate = $identifier; + $identifier = $tag_identifier; } } diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlock.php b/src/applications/files/diff/PhabricatorDocumentEngineBlock.php new file mode 100644 index 0000000000..e2d5af23b2 --- /dev/null +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlock.php @@ -0,0 +1,57 @@ +content = $content; + return $this; + } + + public function getContent() { + return $this->content; + } + + public function setBlockKey($block_key) { + $this->blockKey = $block_key; + return $this; + } + + public function getBlockKey() { + return $this->blockKey; + } + + public function setDifferenceHash($difference_hash) { + $this->differenceHash = $difference_hash; + return $this; + } + + public function getDifferenceHash() { + return $this->differenceHash; + } + + public function setDifferenceType($difference_type) { + $this->differenceType = $difference_type; + return $this; + } + + public function getDifferenceType() { + return $this->differenceType; + } + + public function setIsVisible($is_visible) { + $this->isVisible = $is_visible; + return $this; + } + + public function getIsVisible() { + return $this->isVisible; + } + +} diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php b/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php new file mode 100644 index 0000000000..b582d523c6 --- /dev/null +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlockDiff.php @@ -0,0 +1,47 @@ +oldContent = $old_content; + return $this; + } + + public function getOldContent() { + return $this->oldContent; + } + + public function setNewContent($new_content) { + $this->newContent = $new_content; + return $this; + } + + public function getNewContent() { + return $this->newContent; + } + + public function addOldClass($class) { + $this->oldClasses[] = $class; + return $this; + } + + public function getOldClasses() { + return $this->oldClasses; + } + + public function addNewClass($class) { + $this->newClasses[] = $class; + return $this; + } + + public function getNewClasses() { + return $this->newClasses; + } + +} diff --git a/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php new file mode 100644 index 0000000000..f8f3a414b1 --- /dev/null +++ b/src/applications/files/diff/PhabricatorDocumentEngineBlocks.php @@ -0,0 +1,179 @@ +messages[] = $message; + return $this; + } + + public function getMessages() { + return $this->messages; + } + + public function addBlockList(PhabricatorDocumentRef $ref, array $blocks) { + assert_instances_of($blocks, 'PhabricatorDocumentEngineBlock'); + + $this->lists[] = array( + 'ref' => $ref, + 'blocks' => array_values($blocks), + ); + + return $this; + } + + public function getDocumentRefs() { + return ipull($this->lists, 'ref'); + } + + public function newTwoUpLayout() { + $rows = array(); + $lists = $this->lists; + + if (count($lists) != 2) { + return array(); + } + + $specs = array(); + foreach ($this->lists as $list) { + $specs[] = $this->newDiffSpec($list['blocks']); + } + + $old_map = $specs[0]['map']; + $new_map = $specs[1]['map']; + + $old_list = $specs[0]['list']; + $new_list = $specs[1]['list']; + + $changeset = id(new PhabricatorDifferenceEngine()) + ->generateChangesetFromFileContent($old_list, $new_list); + + $hunk_parser = id(new DifferentialHunkParser()) + ->parseHunksForLineData($changeset->getHunks()) + ->reparseHunksForSpecialAttributes(); + + $hunk_parser->generateVisibleLinesMask(2); + $mask = $hunk_parser->getVisibleLinesMask(); + + $old_lines = $hunk_parser->getOldLines(); + $new_lines = $hunk_parser->getNewLines(); + + $rows = array(); + + $count = count($old_lines); + for ($ii = 0; $ii < $count; $ii++) { + $old_line = idx($old_lines, $ii); + $new_line = idx($new_lines, $ii); + + $is_visible = !empty($mask[$ii + 1]); + + // TODO: There's currently a bug where one-line files get incorrectly + // masked. This causes images to completely fail to render. Just ignore + // the mask if it came back empty. + if (!$mask) { + $is_visible = true; + } + + if ($old_line) { + $old_hash = rtrim($old_line['text'], "\n"); + if (!strlen($old_hash)) { + // This can happen when one of the sources has no blocks. + $old_block = null; + } else { + $old_block = array_shift($old_map[$old_hash]); + $old_block + ->setDifferenceType($old_line['type']) + ->setIsVisible($is_visible); + } + } else { + $old_block = null; + } + + if ($new_line) { + $new_hash = rtrim($new_line['text'], "\n"); + if (!strlen($new_hash)) { + $new_block = null; + } else { + $new_block = array_shift($new_map[$new_hash]); + $new_block + ->setDifferenceType($new_line['type']) + ->setIsVisible($is_visible); + } + } else { + $new_block = null; + } + + // If both lists are empty, we may generate a row which has two empty + // blocks. + if (!$old_block && !$new_block) { + continue; + } + + $rows[] = array( + $old_block, + $new_block, + ); + } + + return $rows; + } + + public function newOneUpLayout() { + $rows = array(); + $lists = $this->lists; + + $idx = 0; + while (true) { + $found_any = false; + + $row = array(); + foreach ($lists as $list) { + $blocks = $list['blocks']; + $cell = idx($blocks, $idx); + + if ($cell !== null) { + $found_any = true; + } + + if ($cell) { + $rows[] = $cell; + } + } + + if (!$found_any) { + break; + } + + $idx++; + } + + return $rows; + } + + + private function newDiffSpec(array $blocks) { + $map = array(); + $list = array(); + + foreach ($blocks as $block) { + $hash = $block->getDifferenceHash(); + + if (!isset($map[$hash])) { + $map[$hash] = array(); + } + $map[$hash][] = $block; + + $list[] = $hash; + } + + return array( + 'map' => $map, + 'list' => implode("\n", $list)."\n", + ); + } + +} diff --git a/src/applications/files/document/PhabricatorDocumentEngine.php b/src/applications/files/document/PhabricatorDocumentEngine.php index c869f5f0d7..bc7960b4ad 100644 --- a/src/applications/files/document/PhabricatorDocumentEngine.php +++ b/src/applications/files/document/PhabricatorDocumentEngine.php @@ -31,6 +31,42 @@ abstract class PhabricatorDocumentEngine return $this->canRenderDocumentType($ref); } + public function canDiffDocuments( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + return false; + } + + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $u_content = $this->newBlockContentView($uref, $ublock); + $v_content = $this->newBlockContentView($vref, $vblock); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('old') + ->addOldClass('old-full') + ->setNewContent($v_content) + ->addNewClass('new') + ->addNewClass('new-full'); + } + + public function newBlockContentView( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngineBlock $block) { + return $block->getContent(); + } + + public function newEngineBlocks( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + throw new PhutilMethodNotImplementedException(); + } + public function canConfigureEncoding(PhabricatorDocumentRef $ref) { return false; } diff --git a/src/applications/files/document/PhabricatorDocumentRef.php b/src/applications/files/document/PhabricatorDocumentRef.php index 036656c00e..12d9f4f2fd 100644 --- a/src/applications/files/document/PhabricatorDocumentRef.php +++ b/src/applications/files/document/PhabricatorDocumentRef.php @@ -11,6 +11,7 @@ final class PhabricatorDocumentRef private $symbolMetadata = array(); private $blameURI; private $coverage = array(); + private $data; public function setFile(PhabricatorFile $file) { $this->file = $file; @@ -65,6 +66,10 @@ final class PhabricatorDocumentRef return $this->byteLength; } + if ($this->data !== null) { + return strlen($this->data); + } + if ($this->file) { return (int)$this->file->getByteSize(); } @@ -72,7 +77,26 @@ final class PhabricatorDocumentRef return null; } + public function setData($data) { + $this->data = $data; + return $this; + } + public function loadData($begin = null, $end = null) { + if ($this->data !== null) { + $data = $this->data; + + if ($begin !== null && $end !== null) { + $data = substr($data, $begin, $end - $begin); + } else if ($begin !== null) { + $data = substr($data, $begin); + } else if ($end !== null) { + $data = substr($data, 0, $end); + } + + return $data; + } + if ($this->file) { $iterator = $this->file->getFileDataIterator($begin, $end); diff --git a/src/applications/files/document/PhabricatorImageDocumentEngine.php b/src/applications/files/document/PhabricatorImageDocumentEngine.php index d0b2099dd0..fa678cc034 100644 --- a/src/applications/files/document/PhabricatorImageDocumentEngine.php +++ b/src/applications/files/document/PhabricatorImageDocumentEngine.php @@ -17,6 +17,71 @@ final class PhabricatorImageDocumentEngine return (1024 * 1024 * 64); } + public function canDiffDocuments( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + + // For now, we can only render a rich image diff if both documents have + // their data stored in Files already. + + return ($uref->getFile() && $vref->getFile()); + } + + public function newEngineBlocks( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + + $u_blocks = $this->newDiffBlocks($uref); + $v_blocks = $this->newDiffBlocks($vref); + + return id(new PhabricatorDocumentEngineBlocks()) + ->addBlockList($uref, $u_blocks) + ->addBlockList($vref, $v_blocks); + } + + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $u_content = $this->newBlockContentView($uref, $ublock); + $v_content = $this->newBlockContentView($vref, $vblock); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('diff-image-cell') + ->setNewContent($v_content) + ->addNewClass('diff-image-cell'); + } + + + private function newDiffBlocks(PhabricatorDocumentRef $ref) { + $blocks = array(); + + $file = $ref->getFile(); + + $image_view = phutil_tag( + 'div', + array( + 'class' => 'differential-image-stage', + ), + phutil_tag( + 'img', + array( + 'src' => $file->getBestURI(), + ))); + + $hash = $file->getContentHash(); + + $blocks[] = id(new PhabricatorDocumentEngineBlock()) + ->setBlockKey('1') + ->setDifferenceHash($hash) + ->setContent($image_view); + + return $blocks; + } + protected function canRenderDocumentType(PhabricatorDocumentRef $ref) { $file = $ref->getFile(); if ($file) { diff --git a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php index 90b9d33c0e..b5c8b04b6d 100644 --- a/src/applications/files/document/PhabricatorJupyterDocumentEngine.php +++ b/src/applications/files/document/PhabricatorJupyterDocumentEngine.php @@ -35,55 +35,198 @@ final class PhabricatorJupyterDocumentEngine return $ref->isProbablyJSON(); } + public function canDiffDocuments( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + return true; + } + + public function newEngineBlocks( + PhabricatorDocumentRef $uref, + PhabricatorDocumentRef $vref) { + + $blocks = new PhabricatorDocumentEngineBlocks(); + + try { + $u_blocks = $this->newDiffBlocks($uref); + $v_blocks = $this->newDiffBlocks($vref); + + $blocks->addBlockList($uref, $u_blocks); + $blocks->addBlockList($vref, $v_blocks); + } catch (Exception $ex) { + $blocks->addMessage($ex->getMessage()); + } + + return $blocks; + } + + public function newBlockDiffViews( + PhabricatorDocumentRef $uref, + PhabricatorDocumentEngineBlock $ublock, + PhabricatorDocumentRef $vref, + PhabricatorDocumentEngineBlock $vblock) { + + $ucell = $ublock->getContent(); + $vcell = $vblock->getContent(); + + $utype = idx($ucell, 'cell_type'); + $vtype = idx($vcell, 'cell_type'); + + if ($utype === $vtype) { + switch ($utype) { + case 'markdown': + $usource = idx($ucell, 'source'); + $usource = implode('', $usource); + + $vsource = idx($vcell, 'source'); + $vsource = implode('', $vsource); + + $diff = id(new PhutilProseDifferenceEngine()) + ->getDiff($usource, $vsource); + + $u_content = $this->newProseDiffCell($diff, array('=', '-')); + $v_content = $this->newProseDiffCell($diff, array('=', '+')); + + $u_content = $this->newJupyterCell(null, $u_content, null); + $v_content = $this->newJupyterCell(null, $v_content, null); + + $u_content = $this->newCellContainer($u_content); + $v_content = $this->newCellContainer($v_content); + + return id(new PhabricatorDocumentEngineBlockDiff()) + ->setOldContent($u_content) + ->addOldClass('old') + ->setNewContent($v_content) + ->addNewClass('new'); + } + } + + return parent::newBlockDiffViews($uref, $ublock, $vref, $vblock); + } + + public function newBlockContentView( + PhabricatorDocumentRef $ref, + PhabricatorDocumentEngineBlock $block) { + + $viewer = $this->getViewer(); + $cell = $block->getContent(); + + $cell_content = $this->renderJupyterCell($viewer, $cell); + + return $this->newCellContainer($cell_content); + } + + private function newCellContainer($cell_content) { + $notebook_table = phutil_tag( + 'table', + array( + 'class' => 'jupyter-notebook', + ), + $cell_content); + + $container = phutil_tag( + 'div', + array( + 'class' => 'document-engine-jupyter document-engine-diff', + ), + $notebook_table); + + return $container; + } + + private function newProseDiffCell(PhutilProseDiff $diff, array $mask) { + $mask = array_fuse($mask); + + $result = array(); + foreach ($diff->getParts() as $part) { + $type = $part['type']; + $text = $part['text']; + + if (!isset($mask[$type])) { + continue; + } + + switch ($type) { + case '-': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'bright', + ), + $text); + break; + case '+': + $result[] = phutil_tag( + 'span', + array( + 'class' => 'bright', + ), + $text); + break; + case '=': + $result[] = $text; + break; + } + } + + return array( + null, + phutil_tag( + 'div', + array( + 'class' => 'jupyter-cell-markdown', + ), + $result), + ); + } + + private function newDiffBlocks(PhabricatorDocumentRef $ref) { + $viewer = $this->getViewer(); + $content = $ref->loadData(); + + $cells = $this->newCells($content, true); + + $idx = 1; + $blocks = array(); + foreach ($cells as $cell) { + // When the cell is a source code line, we can hash just the raw + // input rather than all the cell metadata. + + switch (idx($cell, 'cell_type')) { + case 'code/line': + $hash_input = $cell['raw']; + break; + case 'markdown': + $hash_input = implode('', $cell['source']); + break; + default: + $hash_input = serialize($cell); + break; + } + + $hash = PhabricatorHash::digestWithNamedKey( + $hash_input, + 'document-engine.content-digest'); + + $blocks[] = id(new PhabricatorDocumentEngineBlock()) + ->setBlockKey($idx) + ->setDifferenceHash($hash) + ->setContent($cell); + + $idx++; + } + + return $blocks; + } + protected function newDocumentContent(PhabricatorDocumentRef $ref) { $viewer = $this->getViewer(); $content = $ref->loadData(); try { - $data = phutil_json_decode($content); - } catch (PhutilJSONParserException $ex) { - return $this->newMessage( - pht( - 'This is not a valid JSON document and can not be rendered as '. - 'a Jupyter notebook: %s.', - $ex->getMessage())); - } - - if (!is_array($data)) { - return $this->newMessage( - pht( - 'This document does not encode a valid JSON object and can not '. - 'be rendered as a Jupyter notebook.')); - } - - - $nbformat = idx($data, 'nbformat'); - if (!strlen($nbformat)) { - return $this->newMessage( - pht( - 'This document is missing an "nbformat" field. Jupyter notebooks '. - 'must have this field.')); - } - - if ($nbformat !== 4) { - return $this->newMessage( - pht( - 'This Jupyter notebook uses an unsupported version of the file '. - 'format (found version %s, expected version 4).', - $nbformat)); - } - - $cells = idx($data, 'cells'); - if (!is_array($cells)) { - return $this->newMessage( - pht( - 'This Jupyter notebook does not specify a list of "cells".')); - } - - if (!$cells) { - return $this->newMessage( - pht( - 'This Jupyter notebook does not specify any notebook cells.')); + $cells = $this->newCells($content, false); + } catch (Exception $ex) { + return $this->newMessage($ex->getMessage()); } $rows = array(); @@ -108,20 +251,169 @@ final class PhabricatorJupyterDocumentEngine return $container; } + private function newCells($content, $for_diff) { + try { + $data = phutil_json_decode($content); + } catch (PhutilJSONParserException $ex) { + throw new Exception( + pht( + 'This is not a valid JSON document and can not be rendered as '. + 'a Jupyter notebook: %s.', + $ex->getMessage())); + } + + if (!is_array($data)) { + throw new Exception( + pht( + 'This document does not encode a valid JSON object and can not '. + 'be rendered as a Jupyter notebook.')); + } + + + $nbformat = idx($data, 'nbformat'); + if (!strlen($nbformat)) { + throw new Exception( + pht( + 'This document is missing an "nbformat" field. Jupyter notebooks '. + 'must have this field.')); + } + + if ($nbformat !== 4) { + throw new Exception( + pht( + 'This Jupyter notebook uses an unsupported version of the file '. + 'format (found version %s, expected version 4).', + $nbformat)); + } + + $cells = idx($data, 'cells'); + if (!is_array($cells)) { + throw new Exception( + pht( + 'This Jupyter notebook does not specify a list of "cells".')); + } + + if (!$cells) { + throw new Exception( + pht( + 'This Jupyter notebook does not specify any notebook cells.')); + } + + if (!$for_diff) { + return $cells; + } + + // If we're extracting cells to build a diff view, split code cells into + // individual lines and individual outputs. We want users to be able to + // add inline comments to each line and each output block. + + $results = array(); + foreach ($cells as $cell) { + $cell_type = idx($cell, 'cell_type'); + + if ($cell_type === 'markdown') { + $source = $cell['source']; + $source = implode('', $source); + + // Attempt to split contiguous blocks of markdown into smaller + // pieces. + + $chunks = preg_split( + '/\n\n+/', + $source); + + foreach ($chunks as $chunk) { + $result = $cell; + $result['source'] = array($chunk); + $results[] = $result; + } + + continue; + } + + if ($cell_type !== 'code') { + $results[] = $cell; + continue; + } + + $label = $this->newCellLabel($cell); + + $lines = idx($cell, 'source'); + if (!is_array($lines)) { + $lines = array(); + } + + $content = $this->highlightLines($lines); + + $count = count($lines); + for ($ii = 0; $ii < $count; $ii++) { + $is_head = ($ii === 0); + $is_last = ($ii === ($count - 1)); + + if ($is_head) { + $line_label = $label; + } else { + $line_label = null; + } + + $results[] = array( + 'cell_type' => 'code/line', + 'label' => $line_label, + 'raw' => $lines[$ii], + 'display' => idx($content, $ii), + 'head' => $is_head, + 'last' => $is_last, + ); + } + + $outputs = array(); + $output_list = idx($cell, 'outputs'); + if (is_array($output_list)) { + foreach ($output_list as $output) { + $results[] = array( + 'cell_type' => 'code/output', + 'output' => $output, + ); + } + } + } + + return $results; + } + + private function renderJupyterCell( PhabricatorUser $viewer, array $cell) { list($label, $content) = $this->renderJupyterCellContent($viewer, $cell); + $classes = null; + switch (idx($cell, 'cell_type')) { + case 'code/line': + $classes = 'jupyter-cell-flush'; + break; + } + + return $this->newJupyterCell( + $label, + $content, + $classes); + } + + private function newJupyterCell($label, $content, $classes) { $label_cell = phutil_tag( - 'th', - array(), + 'td', + array( + 'class' => 'jupyter-label', + ), $label); $content_cell = phutil_tag( 'td', - array(), + array( + 'class' => $classes, + ), $content); return phutil_tag( @@ -142,10 +434,15 @@ final class PhabricatorJupyterDocumentEngine case 'markdown': return $this->newMarkdownCell($cell); case 'code': - return $this->newCodeCell($cell); + return $this->newCodeCell($cell); + case 'code/line': + return $this->newCodeLineCell($cell); + case 'code/output': + return $this->newCodeOutputCell($cell); } - return $this->newRawCell(id(new PhutilJSON())->encodeFormatted($cell)); + return $this->newRawCell(id(new PhutilJSON()) + ->encodeFormatted($cell)); } private function newRawCell($content) { @@ -166,8 +463,9 @@ final class PhabricatorJupyterDocumentEngine $content = array(); } - $content = implode('', $content); - $content = phutil_escape_html_newlines($content); + // TODO: This should ideally highlight as Markdown, but the "md" + // highlighter in Pygments is painfully slow and not terribly useful. + $content = $this->highlightLines($content, 'txt'); return array( null, @@ -181,23 +479,14 @@ final class PhabricatorJupyterDocumentEngine } private function newCodeCell(array $cell) { - $execution_count = idx($cell, 'execution_count'); - if ($execution_count) { - $label = 'In ['.$execution_count.']:'; - } else { - $label = null; - } + $label = $this->newCellLabel($cell); $content = idx($cell, 'source'); if (!is_array($content)) { $content = array(); } - $content = implode('', $content); - - $content = PhabricatorSyntaxHighlighter::highlightWithLanguage( - 'py', - $content); + $content = $this->highlightLines($content); $outputs = array(); $output_list = idx($cell, 'outputs'); @@ -213,7 +502,9 @@ final class PhabricatorJupyterDocumentEngine phutil_tag( 'div', array( - 'class' => 'jupyter-cell-code PhabricatorMonospaced remarkup-code', + 'class' => + 'jupyter-cell-code jupyter-cell-code-block '. + 'PhabricatorMonospaced remarkup-code', ), array( $content, @@ -223,6 +514,45 @@ final class PhabricatorJupyterDocumentEngine ); } + private function newCodeLineCell(array $cell) { + $classes = array(); + $classes[] = 'PhabricatorMonospaced'; + $classes[] = 'remarkup-code'; + $classes[] = 'jupyter-cell-code'; + $classes[] = 'jupyter-cell-code-line'; + + if ($cell['head']) { + $classes[] = 'jupyter-cell-code-head'; + } + + if ($cell['last']) { + $classes[] = 'jupyter-cell-code-last'; + } + + $classes = implode(' ', $classes); + + return array( + $cell['label'], + array( + phutil_tag( + 'div', + array( + 'class' => $classes, + ), + array( + $cell['display'], + )), + ), + ); + } + + private function newCodeOutputCell(array $cell) { + return array( + null, + $this->newOutput($cell['output']), + ); + } + private function newOutput(array $output) { if (!is_array($output)) { return pht(''); @@ -309,4 +639,50 @@ final class PhabricatorJupyterDocumentEngine $content); } + private function newCellLabel(array $cell) { + $execution_count = idx($cell, 'execution_count'); + if ($execution_count) { + $label = 'In ['.$execution_count.']:'; + } else { + $label = null; + } + + return $label; + } + + private function highlightLines(array $lines, $force_language = null) { + if ($force_language === null) { + $head = head($lines); + $matches = null; + if (preg_match('/^%%(.*)$/', $head, $matches)) { + $restore = array_shift($lines); + $lang = $matches[1]; + } else { + $restore = null; + $lang = 'py'; + } + } else { + $restore = null; + $lang = $force_language; + } + + $content = PhabricatorSyntaxHighlighter::highlightWithLanguage( + $lang, + implode('', $lines)); + $content = phutil_split_lines($content); + + if ($restore !== null) { + $language_tag = phutil_tag( + 'span', + array( + 'class' => 'language-tag', + ), + $restore); + + array_unshift($content, $language_tag); + } + + return $content; + } + } diff --git a/src/applications/paste/query/PhabricatorPasteQuery.php b/src/applications/paste/query/PhabricatorPasteQuery.php index 54b53ceb4a..693c82780f 100644 --- a/src/applications/paste/query/PhabricatorPasteQuery.php +++ b/src/applications/paste/query/PhabricatorPasteQuery.php @@ -378,15 +378,15 @@ final class PhabricatorPasteQuery } private function highlightSource($source, $title, $language) { - if (empty($language)) { - return PhabricatorSyntaxHighlighter::highlightWithFilename( - $title, - $source); - } else { - return PhabricatorSyntaxHighlighter::highlightWithLanguage( - $language, - $source); - } + if (empty($language)) { + return PhabricatorSyntaxHighlighter::highlightWithFilename( + $title, + $source); + } else { + return PhabricatorSyntaxHighlighter::highlightWithLanguage( + $language, + $source); + } } public function getQueryApplicationClass() { diff --git a/src/applications/people/controller/PhabricatorPeopleRenameController.php b/src/applications/people/controller/PhabricatorPeopleRenameController.php index 42eebfc8ae..276eb8a2c5 100644 --- a/src/applications/people/controller/PhabricatorPeopleRenameController.php +++ b/src/applications/people/controller/PhabricatorPeopleRenameController.php @@ -3,8 +3,13 @@ final class PhabricatorPeopleRenameController extends PhabricatorPeopleController { + public function shouldRequireAdmin() { + return false; + } + public function handleRequest(AphrontRequest $request) { $viewer = $this->getViewer(); + $id = $request->getURIData('id'); $user = id(new PhabricatorPeopleQuery()) @@ -17,6 +22,25 @@ final class PhabricatorPeopleRenameController $done_uri = $this->getApplicationURI("manage/{$id}/"); + if (!$viewer->getIsAdmin()) { + $dialog = $this->newDialog() + ->setTitle(pht('Change Username')) + ->appendParagraph( + pht( + 'You can not change usernames because you are not an '. + 'administrator. Only administrators can change usernames.')) + ->addCancelButton($done_uri, pht('Okay')); + + $message_body = PhabricatorAuthMessage::loadMessageText( + $viewer, + PhabricatorAuthChangeUsernameMessageType::MESSAGEKEY); + if (strlen($message_body)) { + $dialog->appendRemarkup($message_body); + } + + return $dialog; + } + $validation_exception = null; $username = $user->getUsername(); if ($request->isFormOrHisecPost()) { @@ -43,32 +67,25 @@ final class PhabricatorPeopleRenameController } - $inst1 = pht( - 'Be careful when renaming users!'); + $instructions = array(); - $inst2 = pht( - 'The old username will no longer be tied to the user, so anything '. - 'which uses it (like old commit messages) will no longer associate '. - 'correctly. (And, if you give a user a username which some other user '. - 'used to have, username lookups will begin returning the wrong user.)'); + $instructions[] = pht( + 'If you rename this user, the old username will no longer be tied '. + 'to the user account. Anything which uses the old username in raw '. + 'text (like old commit messages) may no longer associate correctly.'); - $inst3 = pht( - 'It is generally safe to rename newly created users (and test users '. - 'and so on), but less safe to rename established users and unsafe to '. - 'reissue a username.'); + $instructions[] = pht( + 'It is generally safe to rename users, but changing usernames may '. + 'create occasional minor complications or confusion with text that '. + 'contains the old username.'); - $inst4 = pht( - 'Users who rely on password authentication will need to reset their '. - 'password after their username is changed (their username is part of '. - 'the salt in the password hash).'); - - $inst5 = pht( + $instructions[] = pht( 'The user will receive an email notifying them that you changed their '. - 'username, with instructions for logging in and resetting their '. - 'password if necessary.'); + 'username.'); + + $instructions[] = null; $form = id(new AphrontFormView()) - ->setUser($viewer) ->appendChild( id(new AphrontFormStaticControl()) ->setLabel(pht('Old Username')) @@ -79,19 +96,20 @@ final class PhabricatorPeopleRenameController ->setValue($username) ->setName('username')); - return $this->newDialog() - ->setWidth(AphrontDialogView::WIDTH_FORM) + $dialog = $this->newDialog() ->setTitle(pht('Change Username')) - ->setValidationException($validation_exception) - ->appendParagraph($inst1) - ->appendParagraph($inst2) - ->appendParagraph($inst3) - ->appendParagraph($inst4) - ->appendParagraph($inst5) - ->appendParagraph(null) + ->setValidationException($validation_exception); + + foreach ($instructions as $instruction) { + $dialog->appendParagraph($instruction); + } + + $dialog ->appendForm($form) ->addSubmitButton(pht('Rename User')) ->addCancelButton($done_uri); + + return $dialog; } } diff --git a/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php b/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php index 72fb683401..f823ead6d9 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryRefEngine.php @@ -10,7 +10,16 @@ final class PhabricatorRepositoryRefEngine private $newPositions = array(); private $deadPositions = array(); private $closeCommits = array(); - private $hasNoCursors; + private $rebuild; + + public function setRebuild($rebuild) { + $this->rebuild = $rebuild; + return $this; + } + + public function getRebuild() { + return $this->rebuild; + } public function updateRefs() { $this->newPositions = array(); @@ -60,15 +69,17 @@ final class PhabricatorRepositoryRefEngine ->execute(); $cursor_groups = mgroup($all_cursors, 'getRefType'); - $this->hasNoCursors = (!$all_cursors); - - // Find all the heads of closing refs. + // Find all the heads of permanent refs. $all_closing_heads = array(); foreach ($all_cursors as $cursor) { - $should_close = $this->shouldCloseRef( - $cursor->getRefType(), - $cursor->getRefName()); - if (!$should_close) { + + // See T13284. Note that we're considering whether this ref was a + // permanent ref or not the last time we updated refs for this + // repository. This allows us to handle things properly when a ref + // is reconfigured from non-permanent to permanent. + + $was_permanent = $cursor->getIsPermanent(); + if (!$was_permanent) { continue; } @@ -76,6 +87,7 @@ final class PhabricatorRepositoryRefEngine $all_closing_heads[] = $identifier; } } + $all_closing_heads = array_unique($all_closing_heads); $all_closing_heads = $this->removeMissingCommits($all_closing_heads); @@ -88,12 +100,18 @@ final class PhabricatorRepositoryRefEngine $this->setCloseFlagOnCommits($this->closeCommits); } - if ($this->newPositions || $this->deadPositions) { + $save_cursors = $this->getCursorsForUpdate($all_cursors); + + if ($this->newPositions || $this->deadPositions || $save_cursors) { $repository->openTransaction(); $this->saveNewPositions(); $this->deleteDeadPositions(); + foreach ($save_cursors as $cursor) { + $cursor->save(); + } + $repository->saveTransaction(); } @@ -103,6 +121,28 @@ final class PhabricatorRepositoryRefEngine } } + private function getCursorsForUpdate(array $cursors) { + assert_instances_of($cursors, 'PhabricatorRepositoryRefCursor'); + + $results = array(); + + foreach ($cursors as $cursor) { + $ref_type = $cursor->getRefType(); + $ref_name = $cursor->getRefName(); + + $is_permanent = $this->isPermanentRef($ref_type, $ref_name); + + if ($is_permanent == $cursor->getIsPermanent()) { + continue; + } + + $cursor->setIsPermanent((int)$is_permanent); + $results[] = $cursor; + } + + return $results; + } + private function updateBranchStates( PhabricatorRepository $repository, array $branches) { @@ -301,11 +341,41 @@ final class PhabricatorRepositoryRefEngine $this->markPositionNew($new_position); } - if ($this->shouldCloseRef($ref_type, $name)) { - foreach ($added_commits as $identifier) { + if ($this->isPermanentRef($ref_type, $name)) { + + // See T13284. If this cursor was already marked as permanent, we + // only need to publish the newly created ref positions. However, if + // this cursor was not previously permanent but has become permanent, + // we need to publish all the ref positions. + + // This corresponds to users reconfiguring a branch to make it + // permanent without pushing any new commits to it. + + $is_rebuild = $this->getRebuild(); + $was_permanent = $ref_cursor->getIsPermanent(); + + if ($is_rebuild || !$was_permanent) { + $update_all = true; + } else { + $update_all = false; + } + + if ($update_all) { + $update_commits = $new_commits; + } else { + $update_commits = $added_commits; + } + + if ($is_rebuild) { + $exclude = array(); + } else { + $exclude = $all_closing_heads; + } + + foreach ($update_commits as $identifier) { $new_identifiers = $this->loadNewCommitIdentifiers( $identifier, - $all_closing_heads); + $exclude); $this->markCloseCommits($new_identifiers); } @@ -334,19 +404,11 @@ final class PhabricatorRepositoryRefEngine } } - private function shouldCloseRef($ref_type, $ref_name) { + private function isPermanentRef($ref_type, $ref_name) { if ($ref_type !== PhabricatorRepositoryRefCursor::TYPE_BRANCH) { return false; } - if ($this->hasNoCursors) { - // If we don't have any cursors, don't close things. Particularly, this - // corresponds to the case where you've just updated to this code on an - // existing repository: we don't want to requeue message steps for every - // commit on a closeable ref. - return false; - } - return $this->getRepository()->isBranchPermanentRef($ref_name); } @@ -505,10 +567,13 @@ final class PhabricatorRepositoryRefEngine $ref_type, $ref_name) { + $is_permanent = $this->isPermanentRef($ref_type, $ref_name); + $cursor = id(new PhabricatorRepositoryRefCursor()) ->setRepositoryPHID($repository->getPHID()) ->setRefType($ref_type) - ->setRefName($ref_name); + ->setRefName($ref_name) + ->setIsPermanent((int)$is_permanent); try { return $cursor->save(); diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php index 8c806edac8..8d3062195c 100644 --- a/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php +++ b/src/applications/repository/management/PhabricatorRepositoryManagementRefsWorkflow.php @@ -14,6 +14,12 @@ final class PhabricatorRepositoryManagementRefsWorkflow 'name' => 'verbose', 'help' => pht('Show additional debugging information.'), ), + array( + 'name' => 'rebuild', + 'help' => pht( + 'Publish commits currently reachable from any permanent ref, '. + 'ignoring the cached ref state.'), + ), array( 'name' => 'repos', 'wildcard' => true, @@ -41,6 +47,7 @@ final class PhabricatorRepositoryManagementRefsWorkflow $engine = id(new PhabricatorRepositoryRefEngine()) ->setRepository($repo) ->setVerbose($args->getArg('verbose')) + ->setRebuild($args->getArg('rebuild')) ->updateRefs(); } diff --git a/src/applications/repository/storage/PhabricatorRepositoryRefCursor.php b/src/applications/repository/storage/PhabricatorRepositoryRefCursor.php index 87f737d86e..91261f2b93 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryRefCursor.php +++ b/src/applications/repository/storage/PhabricatorRepositoryRefCursor.php @@ -19,6 +19,7 @@ final class PhabricatorRepositoryRefCursor protected $refNameHash; protected $refNameRaw; protected $refNameEncoding; + protected $isPermanent; private $repository = self::ATTACHABLE; private $positions = self::ATTACHABLE; @@ -34,6 +35,7 @@ final class PhabricatorRepositoryRefCursor 'refType' => 'text32', 'refNameHash' => 'bytes12', 'refNameEncoding' => 'text16?', + 'isPermanent' => 'bool', ), self::CONFIG_KEY_SCHEMA => array( 'key_ref' => array( diff --git a/src/applications/system/application/PhabricatorSystemApplication.php b/src/applications/system/application/PhabricatorSystemApplication.php index 3fa40b3912..b6cc13050f 100644 --- a/src/applications/system/application/PhabricatorSystemApplication.php +++ b/src/applications/system/application/PhabricatorSystemApplication.php @@ -22,6 +22,7 @@ final class PhabricatorSystemApplication extends PhabricatorApplication { '/services/' => array( 'encoding/' => 'PhabricatorSystemSelectEncodingController', 'highlight/' => 'PhabricatorSystemSelectHighlightController', + 'viewas/' => 'PhabricatorSystemSelectViewAsController', ), '/readonly/' => array( '(?P[^/]+)/' => 'PhabricatorSystemReadOnlyController', diff --git a/src/applications/system/controller/PhabricatorSystemSelectViewAsController.php b/src/applications/system/controller/PhabricatorSystemSelectViewAsController.php new file mode 100644 index 0000000000..19f800f820 --- /dev/null +++ b/src/applications/system/controller/PhabricatorSystemSelectViewAsController.php @@ -0,0 +1,63 @@ +getViewer(); + $v_engine = $request->getStr('engine'); + + if ($request->isFormPost()) { + $result = array('engine' => $v_engine); + return id(new AphrontAjaxResponse())->setContent($result); + } + + $engines = PhabricatorDocumentEngine::getAllEngines(); + + + // TODO: This controller isn't very good because the valid options depend + // on the file being rendered and most of them can't even diff anything, + // and this ref is completely bogus. + + // For now, we just show everything. + $ref = new PhabricatorDocumentRef(); + + $map = array(); + foreach ($engines as $engine) { + $key = $engine->getDocumentEngineKey(); + $label = $engine->getViewAsLabel($ref); + + if (!strlen($label)) { + continue; + } + + $map[$key] = $label; + } + + asort($map); + + $map = array( + '' => pht('(Use Default)'), + ) + $map; + + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendRemarkupInstructions(pht('Choose a document engine to use.')) + ->appendChild( + id(new AphrontFormSelectControl()) + ->setLabel(pht('View As')) + ->setName('engine') + ->setValue($v_engine) + ->setOptions($map)); + + return $this->newDialog() + ->setTitle(pht('Select Document Engine')) + ->appendForm($form) + ->addSubmitButton(pht('Choose Engine')) + ->addCancelButton('/'); + } +} diff --git a/src/applications/transactions/bulk/type/BulkTokenizerParameterType.php b/src/applications/transactions/bulk/type/BulkTokenizerParameterType.php index a48f8877d5..59682412fd 100644 --- a/src/applications/transactions/bulk/type/BulkTokenizerParameterType.php +++ b/src/applications/transactions/bulk/type/BulkTokenizerParameterType.php @@ -22,7 +22,8 @@ final class BulkTokenizerParameterType $template = new AphrontTokenizerTemplateView(); $template_markup = $template->render(); - $datasource = $this->getDatasource(); + $datasource = $this->getDatasource() + ->setViewer($this->getViewer()); return array( 'markup' => (string)hsprintf('%s', $template_markup), diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 0986247454..14143c733a 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -2523,6 +2523,8 @@ abstract class PhabricatorEditEngine } final public function newBulkEditMap() { + $viewer = $this->getViewer(); + $config = $this->loadDefaultConfiguration(); if (!$config) { throw new Exception( @@ -2542,6 +2544,8 @@ abstract class PhabricatorEditEngine continue; } + $bulk_type->setViewer($viewer); + $bulk_label = $type->getBulkEditLabel(); if ($bulk_label === null) { continue; diff --git a/src/docs/user/userguide/remarkup.diviner b/src/docs/user/userguide/remarkup.diviner index d052a9a248..f96c8c7b25 100644 --- a/src/docs/user/userguide/remarkup.diviner +++ b/src/docs/user/userguide/remarkup.diviner @@ -715,6 +715,18 @@ Press {key down down-right right LP} to activate the hadoken technique. > Press {key down down-right right LP} to activate the hadoken technique. +Anchors +======== + +You can use `{anchor #xyz}` to create a document anchor and later link to +it directly with `#xyz` in the URI. + +Headers also automatically create named anchors. + +If you navigate to `#xyz` in your browser location bar, the page will scroll +to the first anchor with "xyz" as a prefix of the anchor name. + + = Fullscreen Mode = Remarkup editors provide a fullscreen composition mode. This can make it easier diff --git a/src/infrastructure/diff/prose/PhutilProseDiff.php b/src/infrastructure/diff/prose/PhutilProseDiff.php new file mode 100644 index 0000000000..df34d64027 --- /dev/null +++ b/src/infrastructure/diff/prose/PhutilProseDiff.php @@ -0,0 +1,292 @@ +parts[] = array( + 'type' => $type, + 'text' => $text, + ); + return $this; + } + + public function getParts() { + return $this->parts; + } + + /** + * Get diff parts, but replace large blocks of unchanged text with "." + * parts representing missing context. + */ + public function getSummaryParts() { + $parts = $this->getParts(); + + $head_key = head_key($parts); + $last_key = last_key($parts); + + $results = array(); + foreach ($parts as $key => $part) { + $is_head = ($key == $head_key); + $is_last = ($key == $last_key); + + switch ($part['type']) { + case '=': + $pieces = $this->splitTextForSummary($part['text']); + + if ($is_head || $is_last) { + $need = 2; + } else { + $need = 3; + } + + // We don't have enough pieces to omit anything, so just continue. + if (count($pieces) < $need) { + $results[] = $part; + break; + } + + if (!$is_head) { + $results[] = array( + 'type' => '=', + 'text' => head($pieces), + ); + } + + $results[] = array( + 'type' => '.', + 'text' => null, + ); + + if (!$is_last) { + $results[] = array( + 'type' => '=', + 'text' => last($pieces), + ); + } + break; + default: + $results[] = $part; + break; + } + } + + return $results; + } + + + public function reorderParts() { + // Reorder sequences of removed and added sections to put all the "-" + // parts together first, then all the "+" parts together. This produces + // a more human-readable result than intermingling them. + + $o_run = array(); + $n_run = array(); + $result = array(); + foreach ($this->parts as $part) { + $type = $part['type']; + switch ($type) { + case '-': + $o_run[] = $part; + break; + case '+': + $n_run[] = $part; + break; + default: + if ($o_run || $n_run) { + foreach ($this->combineRuns($o_run, $n_run) as $merged_part) { + $result[] = $merged_part; + } + $o_run = array(); + $n_run = array(); + } + $result[] = $part; + break; + } + } + + if ($o_run || $n_run) { + foreach ($this->combineRuns($o_run, $n_run) as $part) { + $result[] = $part; + } + } + + // Now, combine consecuitive runs of the same type of change (like a + // series of "-" parts) into a single run. + $combined = array(); + + $last = null; + $last_text = null; + foreach ($result as $part) { + $type = $part['type']; + + if ($last !== $type) { + if ($last !== null) { + $combined[] = array( + 'type' => $last, + 'text' => $last_text, + ); + } + $last_text = null; + $last = $type; + } + + $last_text .= $part['text']; + } + + if ($last_text !== null) { + $combined[] = array( + 'type' => $last, + 'text' => $last_text, + ); + } + + $this->parts = $combined; + + return $this; + } + + private function combineRuns($o_run, $n_run) { + $o_merge = $this->mergeParts($o_run); + $n_merge = $this->mergeParts($n_run); + + // When removed and added blocks share a prefix or suffix, we sometimes + // want to count it as unchanged (for example, if it is whitespace) but + // sometimes want to count it as changed (for example, if it is a word + // suffix like "ing"). Find common prefixes and suffixes of these layout + // characters and emit them as "=" (unchanged) blocks. + + $layout_characters = array( + ' ' => true, + "\n" => true, + '.' => true, + '!' => true, + ',' => true, + '?' => true, + ']' => true, + '[' => true, + '(' => true, + ')' => true, + '<' => true, + '>' => true, + ); + + $o_text = $o_merge['text']; + $n_text = $n_merge['text']; + $o_len = strlen($o_text); + $n_len = strlen($n_text); + $min_len = min($o_len, $n_len); + + $prefix_len = 0; + for ($pos = 0; $pos < $min_len; $pos++) { + $o = $o_text[$pos]; + $n = $n_text[$pos]; + if ($o !== $n) { + break; + } + if (empty($layout_characters[$o])) { + break; + } + $prefix_len++; + } + + $suffix_len = 0; + for ($pos = 0; $pos < ($min_len - $prefix_len); $pos++) { + $o = $o_text[$o_len - ($pos + 1)]; + $n = $n_text[$n_len - ($pos + 1)]; + if ($o !== $n) { + break; + } + if (empty($layout_characters[$o])) { + break; + } + $suffix_len++; + } + + $results = array(); + + if ($prefix_len) { + $results[] = array( + 'type' => '=', + 'text' => substr($o_text, 0, $prefix_len), + ); + } + + if ($prefix_len < $o_len) { + $results[] = array( + 'type' => '-', + 'text' => substr( + $o_text, + $prefix_len, + $o_len - $prefix_len - $suffix_len), + ); + } + + if ($prefix_len < $n_len) { + $results[] = array( + 'type' => '+', + 'text' => substr( + $n_text, + $prefix_len, + $n_len - $prefix_len - $suffix_len), + ); + } + + if ($suffix_len) { + $results[] = array( + 'type' => '=', + 'text' => substr($o_text, -$suffix_len), + ); + } + + return $results; + } + + private function mergeParts(array $parts) { + $text = ''; + $type = null; + foreach ($parts as $part) { + $part_type = $part['type']; + if ($type === null) { + $type = $part_type; + } + if ($type !== $part_type) { + throw new Exception(pht('Can not merge parts of dissimilar types!')); + } + $text .= $part['text']; + } + + return array( + 'type' => $type, + 'text' => $text, + ); + } + + private function splitTextForSummary($text) { + $matches = null; + + $ok = preg_match('/^(\n*[^\n]+)\n/', $text, $matches); + if (!$ok) { + return array($text); + } + + $head = $matches[1]; + $text = substr($text, strlen($head)); + + $ok = preg_match('/\n([^\n]+\n*)\z/', $text, $matches); + if (!$ok) { + return array($text); + } + + $last = $matches[1]; + $text = substr($text, 0, -strlen($last)); + + if (!strlen(trim($text))) { + return array($head, $last); + } else { + return array($head, $text, $last); + } + } + +} diff --git a/src/infrastructure/diff/prose/PhutilProseDifferenceEngine.php b/src/infrastructure/diff/prose/PhutilProseDifferenceEngine.php new file mode 100644 index 0000000000..15af82fabd --- /dev/null +++ b/src/infrastructure/diff/prose/PhutilProseDifferenceEngine.php @@ -0,0 +1,275 @@ +buildDiff($u, $v, 0); + } + + private function buildDiff($u, $v, $level) { + $u_parts = $this->splitCorpus($u, $level); + $v_parts = $this->splitCorpus($v, $level); + + if ($level === 0) { + $diff = $this->newHashDiff($u_parts, $v_parts); + $too_large = false; + } else { + list($diff, $too_large) = $this->newEditDistanceMatrixDiff( + $u_parts, + $v_parts, + $level); + } + + $diff->reorderParts(); + + // If we just built a character-level diff, we're all done and do not + // need to go any deeper. + if ($level == 3) { + return $diff; + } + + $blocks = array(); + $block = null; + foreach ($diff->getParts() as $part) { + $type = $part['type']; + $text = $part['text']; + switch ($type) { + case '=': + if ($block) { + $blocks[] = $block; + $block = null; + } + $blocks[] = array( + 'type' => $type, + 'text' => $text, + ); + break; + case '-': + if (!$block) { + $block = array( + 'type' => '!', + 'old' => '', + 'new' => '', + ); + } + $block['old'] .= $text; + break; + case '+': + if (!$block) { + $block = array( + 'type' => '!', + 'old' => '', + 'new' => '', + ); + } + $block['new'] .= $text; + break; + } + } + + if ($block) { + $blocks[] = $block; + } + + $result = new PhutilProseDiff(); + foreach ($blocks as $block) { + $type = $block['type']; + if ($type == '=') { + $result->addPart('=', $block['text']); + } else { + $old = $block['old']; + $new = $block['new']; + if (!strlen($old) && !strlen($new)) { + // Nothing to do. + } else if (!strlen($old)) { + $result->addPart('+', $new); + } else if (!strlen($new)) { + $result->addPart('-', $old); + } else { + if ($too_large) { + // If this text was too big to diff, don't try to subdivide it. + $result->addPart('-', $old); + $result->addPart('+', $new); + } else { + $subdiff = $this->buildDiff( + $old, + $new, + $level + 1); + + foreach ($subdiff->getParts() as $part) { + $result->addPart($part['type'], $part['text']); + } + } + } + } + } + + $result->reorderParts(); + + return $result; + } + + private function splitCorpus($corpus, $level) { + switch ($level) { + case 0: + // Level 0: Split into paragraphs. + $expr = '/([\n]+)/'; + break; + case 1: + // Level 1: Split into sentences. + $expr = '/([\n,!;?\.]+)/'; + break; + case 2: + // Level 2: Split into words. + $expr = '/(\s+)/'; + break; + case 3: + // Level 3: Split into characters. + return phutil_utf8v_combined($corpus); + } + + $pieces = preg_split($expr, $corpus, -1, PREG_SPLIT_DELIM_CAPTURE); + return $this->stitchPieces($pieces, $level); + } + + private function stitchPieces(array $pieces, $level) { + $results = array(); + $count = count($pieces); + for ($ii = 0; $ii < $count; $ii += 2) { + $result = $pieces[$ii]; + if ($ii + 1 < $count) { + $result .= $pieces[$ii + 1]; + } + + if ($level < 2) { + // Split pieces into separate text and whitespace sections: make one + // piece out of all the whitespace at the beginning, one piece out of + // all the actual text in the middle, and one piece out of all the + // whitespace at the end. + + $matches = null; + preg_match('/^(\s*)(.*?)(\s*)\z/', $result, $matches); + + if (strlen($matches[1])) { + $results[] = $matches[1]; + } + if (strlen($matches[2])) { + $results[] = $matches[2]; + } + if (strlen($matches[3])) { + $results[] = $matches[3]; + } + } else { + $results[] = $result; + } + } + + // If the input ended with a delimiter, we can get an empty final piece. + // Just discard it. + if (last($results) == '') { + array_pop($results); + } + + return $results; + } + + private function newEditDistanceMatrixDiff( + array $u_parts, + array $v_parts, + $level) { + + $matrix = id(new PhutilEditDistanceMatrix()) + ->setMaximumLength(128) + ->setSequences($u_parts, $v_parts) + ->setComputeString(true); + + // For word-level and character-level changes, smooth the output string + // to reduce the choppiness of the diff. + if ($level > 1) { + $matrix->setApplySmoothing(PhutilEditDistanceMatrix::SMOOTHING_FULL); + } + + $u_pos = 0; + $v_pos = 0; + + $edits = $matrix->getEditString(); + $edits_length = strlen($edits); + + $diff = new PhutilProseDiff(); + for ($ii = 0; $ii < $edits_length; $ii++) { + $c = $edits[$ii]; + if ($c == 's') { + $diff->addPart('=', $u_parts[$u_pos]); + $u_pos++; + $v_pos++; + } else if ($c == 'd') { + $diff->addPart('-', $u_parts[$u_pos]); + $u_pos++; + } else if ($c == 'i') { + $diff->addPart('+', $v_parts[$v_pos]); + $v_pos++; + } else if ($c == 'x') { + $diff->addPart('-', $u_parts[$u_pos]); + $diff->addPart('+', $v_parts[$v_pos]); + $u_pos++; + $v_pos++; + } else { + throw new Exception( + pht( + 'Unexpected character ("%s") in edit string.', + $c)); + } + } + + return array($diff, $matrix->didReachMaximumLength()); + } + + private function newHashDiff(array $u_parts, array $v_parts) { + + $u_ref = new PhabricatorDocumentRef(); + $v_ref = new PhabricatorDocumentRef(); + + $u_blocks = $this->newDocumentEngineBlocks($u_parts); + $v_blocks = $this->newDocumentEngineBlocks($v_parts); + + $rows = id(new PhabricatorDocumentEngineBlocks()) + ->addBlockList($u_ref, $u_blocks) + ->addBlockList($v_ref, $v_blocks) + ->newTwoUpLayout(); + + $diff = new PhutilProseDiff(); + foreach ($rows as $row) { + list($u_block, $v_block) = $row; + + if ($u_block && $v_block) { + if ($u_block->getDifferenceType() === '-') { + $diff->addPart('-', $u_block->getContent()); + $diff->addPart('+', $v_block->getContent()); + } else { + $diff->addPart('=', $u_block->getContent()); + } + } else if ($u_block) { + $diff->addPart('-', $u_block->getContent()); + } else { + $diff->addPart('+', $v_block->getContent()); + } + } + + return $diff; + } + + private function newDocumentEngineBlocks(array $parts) { + $blocks = array(); + + foreach ($parts as $part) { + $hash = PhabricatorHash::digestForIndex($part); + + $blocks[] = id(new PhabricatorDocumentEngineBlock()) + ->setContent($part) + ->setDifferenceHash($hash); + } + + return $blocks; + } + +} diff --git a/src/infrastructure/diff/prose/__tests__/PhutilProseDiffTestCase.php b/src/infrastructure/diff/prose/__tests__/PhutilProseDiffTestCase.php new file mode 100644 index 0000000000..88a64de620 --- /dev/null +++ b/src/infrastructure/diff/prose/__tests__/PhutilProseDiffTestCase.php @@ -0,0 +1,246 @@ +assertProseParts( + '', + '', + array(), + pht('Empty')); + + $this->assertProseParts( + "xxx\nyyy", + "xxx\nzzz\nyyy", + array( + "= xxx\n", + "+ zzz\n", + '= yyy', + ), + pht('Add Paragraph')); + + $this->assertProseParts( + "xxx\nzzz\nyyy", + "xxx\nyyy", + array( + "= xxx\n", + "- zzz\n", + '= yyy', + ), + pht('Remove Paragraph')); + + + // Without smoothing, the alogorithm identifies that "shark" and "cat" + // both contain the letter "a" and tries to express this as a very + // fine-grained edit which replaces "sh" with "c" and then "rk" with "t". + // This is technically correct, but it is much easier for human viewers to + // parse if we smooth this into a single removal and a single addition. + + $this->assertProseParts( + 'They say the shark has nine lives.', + 'They say the cat has nine lives.', + array( + '= They say the ', + '- shark', + '+ cat', + '= has nine lives.', + ), + pht('"Shark/cat" word edit smoothenss.')); + + $this->assertProseParts( + 'Rising quickly, she says', + 'Rising quickly, she remarks:', + array( + '= Rising quickly, she ', + '- says', + '+ remarks:', + ), + pht('"Says/remarks" word edit smoothenss.')); + + $this->assertProseParts( + 'See screenshots', + 'Viewed video files', + array( + '- See screenshots', + '+ Viewed video files', + ), + pht('Complete paragraph rewrite.')); + + $this->assertProseParts( + 'xaaax', + 'xbbbx', + array( + '- xaaax', + '+ xbbbx', + ), + pht('Whole word rewrite with common prefix and suffix.')); + + $this->assertProseParts( + ' aaa ', + ' bbb ', + array( + '= ', + '- aaa', + '+ bbb', + '= ', + ), + pht('Whole word rewrite with whitespace prefix and suffix.')); + + $this->assertSummaryProseParts( + "a\nb\nc\nd\ne\nf\ng\nh\n", + "a\nb\nc\nd\nX\nf\ng\nh\n", + array( + '.', + "= d\n", + '- e', + '+ X', + "= \nf", + '.', + ), + pht('Summary diff with middle change.')); + + $this->assertSummaryProseParts( + "a\nb\nc\nd\ne\nf\ng\nh\n", + "X\nb\nc\nd\ne\nf\ng\nh\n", + array( + '- a', + '+ X', + "= \nb", + '.', + ), + pht('Summary diff with head change.')); + + $this->assertSummaryProseParts( + "a\nb\nc\nd\ne\nf\ng\nh\n", + "a\nb\nc\nd\ne\nf\ng\nX\n", + array( + '.', + "= g\n", + '- h', + '+ X', + "= \n", + ), + pht('Summary diff with last change.')); + + $this->assertProseParts( + 'aaa aaa aaa aaa, bbb bbb bbb bbb.', + "aaa aaa aaa aaa, bbb bbb bbb bbb.\n\n- ccc ccc ccc", + array( + '= aaa aaa aaa aaa, bbb bbb bbb bbb.', + "+ \n\n- ccc ccc ccc", + ), + pht('Diff with new trailing content.')); + + $this->assertProseParts( + 'aaa aaa aaa aaa, bbb bbb bbb bbb.', + 'aaa aaa aaa aaa bbb bbb bbb bbb.', + array( + '= aaa aaa aaa aaa', + '- ,', + '= bbb bbb bbb bbb.', + ), + pht('Diff with a removed comma.')); + + $this->assertProseParts( + 'aaa aaa aaa aaa, bbb bbb bbb bbb.', + "aaa aaa aaa aaa bbb bbb bbb bbb.\n\n- ccc ccc ccc!", + array( + '= aaa aaa aaa aaa', + '- ,', + '= bbb bbb bbb bbb.', + "+ \n\n- ccc ccc ccc!", + ), + pht('Diff with a removed comma and new trailing content.')); + + $this->assertProseParts( + '[ ] Walnuts', + '[X] Walnuts', + array( + '= [', + '- ', + '+ X', + '= ] Walnuts', + ), + pht('Diff adding a tickmark to a checkbox list.')); + + $this->assertProseParts( + '[[ ./week49 ]]', + '[[ ./week50 ]]', + array( + '= [[ ./week', + '- 49', + '+ 50', + '= ]]', + ), + pht('Diff changing a remarkup wiki link target.')); + + // Create a large corpus with many sentences and paragraphs. + $large_paragraph = 'xyz. '; + $large_paragraph = str_repeat($large_paragraph, 50); + $large_paragraph = rtrim($large_paragraph); + + $large_corpus = $large_paragraph."\n\n"; + $large_corpus = str_repeat($large_corpus, 50); + $large_corpus = rtrim($large_corpus); + + $this->assertProseParts( + $large_corpus, + "aaa\n\n".$large_corpus."\n\nzzz", + array( + "+ aaa\n\n", + '= '.$large_corpus, + "+ \n\nzzz", + ), + pht('Adding initial and final lines to a large corpus.')); + + } + + private function assertProseParts($old, $new, array $expect_parts, $label) { + $engine = new PhutilProseDifferenceEngine(); + $diff = $engine->getDiff($old, $new); + + $parts = $diff->getParts(); + + $this->assertParts($expect_parts, $parts, $label); + } + + private function assertSummaryProseParts( + $old, + $new, + array $expect_parts, + $label) { + + $engine = new PhutilProseDifferenceEngine(); + $diff = $engine->getDiff($old, $new); + + $parts = $diff->getSummaryParts(); + + $this->assertParts($expect_parts, $parts, $label); + } + + private function assertParts( + array $expect, + array $actual_parts, + $label) { + + $actual = array(); + foreach ($actual_parts as $actual_part) { + $type = $actual_part['type']; + $text = $actual_part['text']; + + switch ($type) { + case '.': + $actual[] = $type; + break; + default: + $actual[] = "{$type} {$text}"; + break; + } + } + + $this->assertEqual($expect, $actual, $label); + } + + +} diff --git a/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php b/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php index fe5cab8622..02477186ff 100644 --- a/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php +++ b/src/infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php @@ -17,7 +17,7 @@ final class PHUIDiffOneUpInlineCommentRowScaffold $inline = head($inlines); $attrs = array( - 'colspan' => 3, + 'colspan' => 2, 'id' => $inline->getScaffoldCellID(), ); @@ -32,6 +32,7 @@ final class PHUIDiffOneUpInlineCommentRowScaffold $cells = array( phutil_tag('td', array('class' => 'n'), $left_hidden), phutil_tag('td', array('class' => 'n'), $right_hidden), + phutil_tag('td', array('class' => 'copy')), phutil_tag('td', $attrs, $inline), ); diff --git a/src/infrastructure/graph/ManiphestTaskGraph.php b/src/infrastructure/graph/ManiphestTaskGraph.php index 74a1fe8701..5aef7b102a 100644 --- a/src/infrastructure/graph/ManiphestTaskGraph.php +++ b/src/infrastructure/graph/ManiphestTaskGraph.php @@ -84,9 +84,18 @@ final class ManiphestTaskGraph ' ', $link, ); + + $subtype_tag = null; + + $subtype = $object->newSubtypeObject(); + if ($subtype && $subtype->hasTagView()) { + $subtype_tag = $subtype->newTagView() + ->setSlimShady(true); + } } else { $status = null; $assigned = null; + $subtype_tag = null; $link = $viewer->renderHandle($phid); } @@ -115,18 +124,23 @@ final class ManiphestTaskGraph $marker, $trace, $status, + $subtype_tag, $assigned, $link, ); } protected function newTable(AphrontTableView $table) { + $subtype_map = id(new ManiphestTask())->newEditEngineSubtypeMap(); + $has_subtypes = ($subtype_map->getCount() > 1); + return $table ->setHeaders( array( null, null, pht('Status'), + pht('Subtype'), pht('Assigned'), pht('Task'), )) @@ -136,12 +150,15 @@ final class ManiphestTaskGraph 'threads', 'graph-status', null, + null, 'wide pri object-link', )) ->setColumnVisibility( array( true, !$this->getRenderOnlyAdjacentNodes(), + true, + $has_subtypes, )) ->setDeviceVisibility( array( @@ -150,6 +167,11 @@ final class ManiphestTaskGraph // On mobile, we only show the actual graph drawing if we're on the // standalone page, since it can take over the screen otherwise. $this->getIsStandalone(), + true, + + // On mobile, don't show subtypes since they're relatively less + // important and we're more pressured for space. + false, )); } @@ -180,6 +202,7 @@ final class ManiphestTaskGraph null, null, null, + null, pht("\xC2\xB7 \xC2\xB7 \xC2\xB7"), ); } diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index b9467e549d..35de846650 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -539,6 +539,7 @@ final class PhabricatorMarkupEngine extends Phobject { $rules[] = new PhutilRemarkupDelRule(); $rules[] = new PhutilRemarkupUnderlineRule(); $rules[] = new PhutilRemarkupHighlightRule(); + $rules[] = new PhutilRemarkupAnchorRule(); foreach (self::loadCustomInlineRules() as $rule) { $rules[] = clone $rule; diff --git a/src/infrastructure/markup/__tests__/PhabricatorAnchorTestCase.php b/src/infrastructure/markup/__tests__/PhabricatorAnchorTestCase.php new file mode 100644 index 0000000000..2b06fd693d --- /dev/null +++ b/src/infrastructure/markup/__tests__/PhabricatorAnchorTestCase.php @@ -0,0 +1,38 @@ + '', + 'Bells and Whistles' => 'bells-and-whistles', + 'Termination for Nonpayment' => 'termination-for-nonpayment', + $low_ascii => '0123456789-abcdefghijklmnopqrstu', + 'xxxx xxxx xxxx xxxx xxxx on' => 'xxxx-xxxx-xxxx-xxxx-xxxx', + 'xxxx xxxx xxxx xxxx xxxx ox' => 'xxxx-xxxx-xxxx-xxxx-xxxx-ox', + "So, You Want To Build A {$snowman}?" => + "so-you-want-to-build-a-{$snowman}", + str_repeat($snowman, 128) => str_repeat($snowman, 32), + ); + + foreach ($map as $input => $expect) { + $anchor = PhutilRemarkupHeaderBlockRule::getAnchorNameFromHeaderText( + $input); + + $this->assertEqual( + $expect, + $anchor, + pht('Anchor for "%s".', $input)); + } + } + +} diff --git a/src/infrastructure/markup/blockrule/PhutilRemarkupHeaderBlockRule.php b/src/infrastructure/markup/blockrule/PhutilRemarkupHeaderBlockRule.php index cbcb143339..7623a20e59 100644 --- a/src/infrastructure/markup/blockrule/PhutilRemarkupHeaderBlockRule.php +++ b/src/infrastructure/markup/blockrule/PhutilRemarkupHeaderBlockRule.php @@ -73,24 +73,7 @@ final class PhutilRemarkupHeaderBlockRule extends PhutilRemarkupBlockRule { } private function generateAnchor($level, $text) { - $anchor = strtolower($text); - $anchor = preg_replace('/[^a-z0-9]/', '-', $anchor); - $anchor = preg_replace('/--+/', '-', $anchor); - $anchor = trim($anchor, '-'); - $anchor = substr($anchor, 0, 24); - $anchor = trim($anchor, '-'); - $base = $anchor; - - $key = self::KEY_HEADER_TOC; $engine = $this->getEngine(); - $anchors = $engine->getTextMetadata($key, array()); - - $suffix = 1; - while (!strlen($anchor) || isset($anchors[$anchor])) { - $anchor = $base.'-'.$suffix; - $anchor = trim($anchor, '-'); - $suffix++; - } // When a document contains a link inside a header, like this: // @@ -100,12 +83,30 @@ final class PhutilRemarkupHeaderBlockRule extends PhutilRemarkupBlockRule { // header itself. We push the 'toc' state so all the link rules generate // just names. $engine->pushState('toc'); - $text = $this->applyRules($text); - $text = $engine->restoreText($text); - - $anchors[$anchor] = array($level, $text); + $plain_text = $text; + $plain_text = $this->applyRules($plain_text); + $plain_text = $engine->restoreText($plain_text); $engine->popState('toc'); + $anchor = self::getAnchorNameFromHeaderText($plain_text); + + if (!strlen($anchor)) { + return null; + } + + $base = $anchor; + + $key = self::KEY_HEADER_TOC; + $anchors = $engine->getTextMetadata($key, array()); + + $suffix = 1; + while (isset($anchors[$anchor])) { + $anchor = $base.'-'.$suffix; + $anchor = trim($anchor, '-'); + $suffix++; + } + + $anchors[$anchor] = array($level, $plain_text); $engine->setTextMetadata($key, $anchors); return phutil_tag( @@ -159,4 +160,26 @@ final class PhutilRemarkupHeaderBlockRule extends PhutilRemarkupBlockRule { return phutil_implode_html("\n", $toc); } + public static function getAnchorNameFromHeaderText($text) { + $anchor = phutil_utf8_strtolower($text); + $anchor = PhutilRemarkupAnchorRule::normalizeAnchor($anchor); + + // Truncate the fragment to something reasonable. + $anchor = id(new PhutilUTF8StringTruncator()) + ->setMaximumGlyphs(32) + ->setTerminator('') + ->truncateString($anchor); + + // If the fragment is terminated by a word which "The U.S. Government + // Printing Office Style Manual" normally discourages capitalizing in + // titles, discard it. This is an arbitrary heuristic intended to avoid + // awkward hanging words in anchors. + $anchor = preg_replace( + '/-(a|an|the|at|by|for|in|of|on|per|to|up|and|as|but|if|or|nor)\z/', + '', + $anchor); + + return $anchor; + } + } diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupAnchorRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupAnchorRule.php new file mode 100644 index 0000000000..8bb581c2e4 --- /dev/null +++ b/src/infrastructure/markup/markuprule/PhutilRemarkupAnchorRule.php @@ -0,0 +1,69 @@ +getEngine(); + + if ($engine->isTextMode()) { + return null; + } + + if ($engine->isHTMLMailMode()) { + return null; + } + + if ($engine->isAnchorMode()) { + return null; + } + + if (!$this->isFlatText($matches[0])) { + return $matches[0]; + } + + if (!self::isValidAnchorName($matches[1])) { + return $matches[0]; + } + + $tag_view = phutil_tag( + 'a', + array( + 'name' => $matches[1], + ), + ''); + + return $this->getEngine()->storeText($tag_view); + } + + public static function isValidAnchorName($anchor_name) { + $normal_anchor = self::normalizeAnchor($anchor_name); + + if ($normal_anchor === $anchor_name) { + return true; + } + + return false; + } + + public static function normalizeAnchor($anchor) { + // Replace all latin characters which are not "a-z" or "0-9" with "-". + // Preserve other characters, since non-latin letters and emoji work + // fine in anchors. + $anchor = preg_replace('/[\x00-\x2F\x3A-\x60\x7B-\x7F]+/', '-', $anchor); + $anchor = trim($anchor, '-'); + + return $anchor; + } + +} diff --git a/src/infrastructure/markup/markuprule/PhutilRemarkupBoldRule.php b/src/infrastructure/markup/markuprule/PhutilRemarkupBoldRule.php index 4640df2b2e..6d056db0a0 100644 --- a/src/infrastructure/markup/markuprule/PhutilRemarkupBoldRule.php +++ b/src/infrastructure/markup/markuprule/PhutilRemarkupBoldRule.php @@ -18,6 +18,10 @@ final class PhutilRemarkupBoldRule extends PhutilRemarkupRule { } protected function applyCallback(array $matches) { + if ($this->getEngine()->isAnchorMode()) { + return $matches[1]; + } + return hsprintf('%s', $matches[1]); } diff --git a/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php b/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php index 1c5ff78607..0b01fd6a69 100644 --- a/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php +++ b/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php @@ -34,6 +34,10 @@ final class PhutilRemarkupEngine extends PhutilMarkupEngine { return $this->mode & self::MODE_TEXT; } + public function isAnchorMode() { + return $this->getState('toc'); + } + public function isHTMLMailMode() { return $this->mode & self::MODE_HTML_MAIL; } diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt index 43448f75b5..9505271576 100644 --- a/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/toc.txt @@ -6,14 +6,14 @@ ~~~~~~~~~~ -

link_name

+

link_name

bold

diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css index 233ac4cca5..06d5a50721 100644 --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -63,6 +63,11 @@ padding: 1px 8px; } +.differential-diff td.diff-flush { + padding-top: 0; + padding-bottom: 0; +} + .device .differential-diff td { padding: 1px 4px; } @@ -282,18 +287,16 @@ td.cov-I { font-weight: bold; } -.differential-diff .differential-image-diff { +.differential-diff td.diff-image-cell { + background-color: transparent; background-image: url(/rsrc/image/checker_light.png); -} - -.differential-diff .differential-image-diff:hover { - background-image: url(/rsrc/image/checker_dark.png); -} - -.differential-diff .differential-image-diff td { padding: 8px; } +.device-desktop .differential-diff .diff-image-cell:hover { + background-image: url(/rsrc/image/checker_dark.png); +} + .differential-image-stage { overflow: auto; } diff --git a/webroot/rsrc/css/core/syntax.css b/webroot/rsrc/css/core/syntax.css index 90f2981ba6..0114d3fb24 100644 --- a/webroot/rsrc/css/core/syntax.css +++ b/webroot/rsrc/css/core/syntax.css @@ -6,6 +6,10 @@ color: #aa0066; } +.remarkup-code .language-tag { + color: {$lightgreytext}; +} + .remarkup-code td > span { display: inline; word-break: break-all; diff --git a/webroot/rsrc/css/phui/phui-property-list-view.css b/webroot/rsrc/css/phui/phui-property-list-view.css index ed7fb95a7b..98cb0db820 100644 --- a/webroot/rsrc/css/phui/phui-property-list-view.css +++ b/webroot/rsrc/css/phui/phui-property-list-view.css @@ -268,6 +268,10 @@ div.phui-property-list-stacked .phui-property-list-properties margin: 20px; } +.document-engine-jupyter.document-engine-diff { + margin: 0; +} + .document-engine-in-flight { opacity: 0.25; } @@ -294,22 +298,66 @@ div.phui-property-list-stacked .phui-property-list-properties .jupyter-cell-code { white-space: pre-wrap; + word-break: break-word; background: {$lightgreybackground}; - padding: 8px; - border: 1px solid {$lightgreyborder}; border-radius: 2px; + border-color: {$lightgreyborder}; + border-style: solid; +} + +.jupyter-cell-code-block { + padding: 8px; + border-width: 1px; +} + +.jupyter-cell-code-line { + padding: 2px 8px; + border-width: 0 1px; +} + +td.new .jupyter-cell-code-line { + background: {$new-background}; + border-color: {$new-bright}; +} + +td.old .jupyter-cell-code-line { + background: {$old-background}; + border-color: {$old-bright}; +} + +.jupyter-cell-code-head { + border-top-width: 1px; + margin-top: 4px; + padding-top: 8px; +} + +.jupyter-cell-code-last { + border-bottom-width: 1px; + margin-bottom: 4px; + padding-bottom: 8px; } -.jupyter-notebook > tbody > tr > th, .jupyter-notebook > tbody > tr > td { padding: 8px; } -.jupyter-notebook > tbody > tr > th { +.jupyter-notebook > tbody > tr > td.jupyter-cell-flush { + padding-top: 0; + padding-bottom: 0; +} + +.jupyter-notebook, +.jupyter-notebook > tbody > tr > td { + width: 100%; +} + +.jupyter-notebook > tbody > tr > td.jupyter-label { white-space: nowrap; text-align: right; - min-width: 48px; + min-width: 56px; font-weight: bold; + width: auto; + padding: 8px 8px 0; } .jupyter-output { @@ -326,3 +374,7 @@ div.phui-property-list-stacked .phui-property-list-properties .jupyter-output-html { background: {$sh-indigobackground}; } + +.jupyter-cell-markdown { + white-space: pre-wrap; +} diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js index 754f3b16e4..9a1e5d12c2 100644 --- a/webroot/rsrc/js/application/diff/DiffChangeset.js +++ b/webroot/rsrc/js/application/diff/DiffChangeset.js @@ -24,6 +24,7 @@ JX.install('DiffChangeset', { this._ref = data.ref; this._renderer = data.renderer; this._highlight = data.highlight; + this._documentEngine = data.documentEngine; this._encoding = data.encoding; this._loaded = data.loaded; this._treeNodeID = data.treeNodeID; @@ -47,6 +48,7 @@ JX.install('DiffChangeset', { _ref: null, _renderer: null, _highlight: null, + _documentEngine: null, _encoding: null, _undoTemplates: null, @@ -310,6 +312,7 @@ JX.install('DiffChangeset', { ref: this._ref, renderer: this.getRenderer() || '', highlight: this._highlight || '', + engine: this._documentEngine || '', encoding: this._encoding || '' }; }, @@ -366,6 +369,14 @@ JX.install('DiffChangeset', { return this._highlight; }, + setDocumentEngine: function(engine) { + this._documentEngine = engine; + }, + + getDocumentEngine: function(engine) { + return this._documentEngine; + }, + getSelectableItems: function() { var items = []; diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js index 572faad987..341590ea85 100644 --- a/webroot/rsrc/js/application/diff/DiffChangesetList.js +++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js @@ -827,6 +827,26 @@ JX.install('DiffChangesetList', { }); list.addItem(highlight_item); + var engine_item = new JX.PHUIXActionView() + .setIcon('fa-file-image-o') + .setName(pht('View As...')) + .setHandler(function(e) { + var params = { + engine: changeset.getDocumentEngine(), + }; + + new JX.Workflow('/services/viewas/', params) + .setHandler(function(r) { + changeset.setDocumentEngine(r.engine); + changeset.reload(); + }) + .start(); + + e.prevent(); + menu.close(); + }); + list.addItem(engine_item); + add_link('fa-arrow-left', pht('Show Raw File (Left)'), data.leftURI); add_link('fa-arrow-right', pht('Show Raw File (Right)'), data.rightURI); add_link('fa-pencil', pht('Open in Editor'), data.editor, true); @@ -860,6 +880,7 @@ JX.install('DiffChangesetList', { encoding_item.setDisabled(!changeset.isLoaded()); highlight_item.setDisabled(!changeset.isLoaded()); + engine_item.setDisabled(!changeset.isLoaded()); if (changeset.isLoaded()) { if (changeset.getRenderer() == '2up') { @@ -1174,30 +1195,26 @@ JX.install('DiffChangesetList', { bot = tmp; } - // Find the leftmost cell that we're going to highlight: this is the next - // in the row. In 2up views, it should be directly adjacent. In - // 1up views, we may have to skip over the other line number column. - var l = top; - while (JX.DOM.isType(l, 'th')) { - l = l.nextSibling; + // Find the leftmost cell that we're going to highlight. This is the + // next sibling with a "data-copy-mode" attribute, which is a marker + // for the cell with actual content in it. + var content_cell = top; + while (content_cell && !content_cell.getAttribute('data-copy-mode')) { + content_cell = content_cell.nextSibling; } - // Find the rightmost cell that we're going to highlight: this is the - // farthest consecutive, adjacent in the row. Sometimes the left - // and right nodes are the same (left side of 2up view); sometimes we're - // going to highlight several nodes (copy + code + coverage). - var r = l; - while (r.nextSibling && JX.DOM.isType(r.nextSibling, 'td')) { - r = r.nextSibling; + // If we didn't find a cell to highlight, don't highlight anything. + if (!content_cell) { + return; } - var pos = JX.$V(l) - .add(JX.Vector.getAggregateScrollForNode(l)); + var pos = JX.$V(content_cell) + .add(JX.Vector.getAggregateScrollForNode(content_cell)); - var dim = JX.$V(r) - .add(JX.Vector.getAggregateScrollForNode(r)) + var dim = JX.$V(content_cell) + .add(JX.Vector.getAggregateScrollForNode(content_cell)) .add(-pos.x, -pos.y) - .add(JX.Vector.getDim(r)); + .add(JX.Vector.getDim(content_cell)); var bpos = JX.$V(bot) .add(JX.Vector.getAggregateScrollForNode(bot)); diff --git a/webroot/rsrc/js/core/behavior-watch-anchor.js b/webroot/rsrc/js/core/behavior-watch-anchor.js index 49cac764ae..4f8c6b93b4 100644 --- a/webroot/rsrc/js/core/behavior-watch-anchor.js +++ b/webroot/rsrc/js/core/behavior-watch-anchor.js @@ -8,52 +8,102 @@ JX.behavior('phabricator-watch-anchor', function() { - var highlighted; + // When the user loads a page with an "#anchor" or changes the "#anchor" on + // an existing page, we try to scroll the page to the relevant location. - function highlight() { - highlighted && JX.DOM.alterClass(highlighted, 'anchor-target', false); - try { - highlighted = JX.$('anchor-' + window.location.hash.replace('#', '')); - } catch (ex) { - highlighted = null; - } - highlighted && JX.DOM.alterClass(highlighted, 'anchor-target', true); - } + // Browsers do this on their own, but we have some additional rules to try + // to match anchors more flexibly and handle cases where an anchor is not + // yet present in the document because something is still loading or + // rendering it, often via Ajax. - // Defer invocation so other listeners can update the document. - function defer_highlight() { - setTimeout(highlight, 0); - } + // Number of milliseconds we'll keep trying to find an anchor for. + var wait_max = 5000; + + // Wait between retries. + var wait_ms = 100; + + var target; + var retry_ms; - // In some cases, we link to an anchor but the anchor target ajaxes in - // later. If it pops in within the first few seconds, jump to it. function try_anchor() { + retry_ms = wait_max; + seek_anchor(); + } + + function seek_anchor() { var anchor = window.location.hash.replace('#', ''); - try { - // If the anchor exists, assume the browser handled the jump. - if (anchor) { - JX.$(anchor); + + if (!anchor.length) { + return; + } + + var ii; + var node = null; + + // When the user navigates to "#abc", we'll try to find a node with + // either ID "abc" or ID "anchor-abc". + var ids = [anchor, 'anchor-' + anchor]; + + for (ii = 0; ii < ids.length; ii++) { + try { + node = JX.$(ids[ii]); + break; + } catch (e) { + // Continue. } - defer_highlight(); - } catch (e) { - var n = 50; - var try_anchor_again = function () { - try { - var node = JX.$(anchor); - var pos = JX.Vector.getPosWithScroll(node); - JX.DOM.scrollToPosition(0, pos.y - 60); - defer_highlight(); - } catch (e) { - if (n--) { - setTimeout(try_anchor_again, 100); - } + } + + // If we haven't found a matching node yet, look for an "" tag with + // a "name" attribute that has our anchor as a prefix. For example, you + // can navigate to "#cat" and we'll match "#cat-and-mouse". + + if (!node) { + var anchor_nodes = JX.DOM.scry(document.body, 'a'); + for (ii = 0; ii < anchor_nodes.length; ii++) { + if (!anchor_nodes[ii].name) { + continue; } - }; - try_anchor_again(); + + if (anchor_nodes[ii].name.substring(0, anchor.length) === anchor) { + node = anchor_nodes[ii]; + break; + } + } + } + + // If we already have an anchor highlighted, unhighlight it and throw + // it away if it doesn't match the new target. + if (target && (target !== node)) { + JX.DOM.alterClass(target, 'anchor-target', false); + target = null; + } + + // If we didn't find a matching anchor, try again soon. This allows + // rendering logic some time to complete Ajax requests and draw elements + // onto the page. + if (!node) { + if (retry_ms > 0) { + retry_ms -= wait_ms; + setTimeout(try_anchor, wait_ms); + return; + } + } + + // If we've found a new target, highlight it. + if (target !== node) { + target = node; + JX.DOM.alterClass(target, 'anchor-target', true); + } + + // Try to scroll to the new target. + try { + var pos = JX.Vector.getPosWithScroll(node); + JX.DOM.scrollToPosition(0, pos.y - 60); + } catch (e) { + // Ignore issues with scrolling the document. } } JX.Stratcom.listen('hashchange', null, try_anchor); try_anchor(); - });