diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -465,7 +465,7 @@ 'rsrc/js/core/behavior-keyboard-pager.js' => '1325b731', 'rsrc/js/core/behavior-keyboard-shortcuts.js' => '42c44e8b', 'rsrc/js/core/behavior-lightbox-attachments.js' => '14c7ab36', - 'rsrc/js/core/behavior-line-linker.js' => '0d915ff5', + 'rsrc/js/core/behavior-line-linker.js' => '8cbbcfc5', 'rsrc/js/core/behavior-linked-container.js' => '74446546', 'rsrc/js/core/behavior-more.js' => '506aa3f4', 'rsrc/js/core/behavior-object-selector.js' => '98ef467f', @@ -627,7 +627,7 @@ 'javelin-behavior-phabricator-gesture-example' => '242dedd0', 'javelin-behavior-phabricator-keyboard-pager' => '1325b731', 'javelin-behavior-phabricator-keyboard-shortcuts' => '42c44e8b', - 'javelin-behavior-phabricator-line-linker' => '0d915ff5', + 'javelin-behavior-phabricator-line-linker' => '8cbbcfc5', 'javelin-behavior-phabricator-notification-example' => '29819b75', 'javelin-behavior-phabricator-object-selector' => '98ef467f', 'javelin-behavior-phabricator-oncopy' => 'da8f5259', @@ -984,13 +984,6 @@ ), '0d2490ce' => array( 'javelin-install', - ), - '0d915ff5' => array( - 'javelin-behavior', - 'javelin-stratcom', - 'javelin-dom', - 'javelin-history', - 'javelin-external-editor-link-engine', ), '0eaa33a9' => array( 'javelin-behavior', @@ -1668,6 +1661,13 @@ 'phabricator-title', 'phabricator-shaped-request', 'conpherence-thread-manager', + ), + '8cbbcfc5' => array( + 'javelin-behavior', + 'javelin-stratcom', + 'javelin-dom', + 'javelin-history', + 'javelin-external-editor-link-engine', ), '8e0aa661' => array( 'javelin-install', diff --git a/src/view/layout/PhabricatorSourceCodeView.php b/src/view/layout/PhabricatorSourceCodeView.php --- a/src/view/layout/PhabricatorSourceCodeView.php +++ b/src/view/layout/PhabricatorSourceCodeView.php @@ -131,8 +131,9 @@ } if ($this->canClickHighlight) { + $line_id = 'L'.$line_number; if ($base_uri) { - $line_href = $base_uri.'$'.$line_number; + $line_href = $base_uri.'#'.$line_id; } else { $line_href = null; } @@ -142,6 +143,7 @@ array( 'href' => $line_href, 'data-n' => $line_number, + 'id' => $line_id, )); } else { $tag_number = phutil_tag( diff --git a/webroot/rsrc/js/core/behavior-line-linker.js b/webroot/rsrc/js/core/behavior-line-linker.js --- a/webroot/rsrc/js/core/behavior-line-linker.js +++ b/webroot/rsrc/js/core/behavior-line-linker.js @@ -130,6 +130,57 @@ } }; + /** + * Get a valid line number (int) from a string, or scream violently. + * + * @param {String} vRaw String containing a number, like '1'. + * @return {Integer} Integer like 1. + * @throws Do not accept zero. Do not accept negative numbers. + */ + var _parseLineNumber = function(vRaw) { + var v = parseInt(vRaw); + if (isNaN(v) || v <= 0) { + throw 'Input fragment parts must be positive integer. Got: ' + vRaw; + } + return v; + }; + + /** + * Parse the highlighted lines from web fragment, or scream violently. + * + * @param {String} Input string like 'L123' or 'L123-124'. + * @return {Array} Array with always 2 elements: min and max line. + * From the fragment '#L123' you get the array [123, 123]. + * From the fragment '#L123-124' you get the array [123, 124]. + * From the fragment '#L123-123' you get the array [123, 123]. + * @throws Do not accept trash like '#Labc', '#L123-456-789', '#L123-abc'. + */ + var parseMinMaxSelectedLineFromFragment = function(input) { + // The web fragment must be 'L123' or 'L123-124' or similar. + if (!input || input.charAt(0) !== 'L') { + throw 'Input fragment is not a line fragment.'; + } + + // Strip the 'L' and parse the '-' interval (if any). + var linesStr = input.substring(1); + var lines = linesStr.split('-', 2); + var hasOne = lines.length === 1; + var hasTwo = lines.length === 2; + if (!hasOne && !hasTwo) { + throw 'Input fragment must be valid, like L123 or L123-456.'; + } + + // Require valid integers. + var a = _parseLineNumber(lines[0]); + var b = hasTwo ? _parseLineNumber(lines[1]) : a; + + // Sort interval. Avoid dumb JavaScript sort() that returns strings. + if (a < b) { + return [a, b]; + } + return [b, a]; + }; + JX.Stratcom.listen('mouseover', 'phabricator-source', highlight); JX.Stratcom.listen( @@ -151,6 +202,8 @@ if (!uri) { uri = JX.$U(window.location); path = uri.getPath(); + + // Cleanup legacy URIs using '$123' to highlight that line. path = path.replace(/\$[\d-]+$/, ''); uri.setPath(path); uri = uri.toString(); @@ -160,11 +213,15 @@ target = null; root = null; - var lines = (o == t ? o : Math.min(o, t) + '-' + Math.max(o, t)); - uri = JX.$U(uri); path = uri.getPath(); - path = path + '$' + lines; + + // Check if we should highlight a single line or an interval. + // Refresh the web fragment. + var lineInterval = (o == t) ? o : Math.min(o, t) + '-' + Math.max(o, t); + var lineIdentifier = 'L' + lineInterval; + uri.setFragment(lineIdentifier); + uri = uri.setPath(path).toString(); JX.History.replace(uri); @@ -188,9 +245,35 @@ }); - // Try to jump to the highlighted lines if we don't have an explicit anchor - // in the URI. - if (!window.location.hash.length) { + // Try to jump to the highlighted lines at startup. + if (window.location.hash.length) { + // Parse the web fragment '#L123' or '#L123-124' and highlight that. + var currentFragment = JX.$U(window.location).getFragment(); + try { + var lines = parseMinMaxSelectedLineFromFragment(currentFragment); + var minLine = lines[0]; + var maxLine = lines[1]; + + // Scroll to the very first line. + var lineNode = JX.$('L' + minLine); + var tr = JX.DOM.findAbove(lineNode, 'tr'); + JX.DOM.scrollToPosition(0, JX.$V(tr).y - 60); + + // Highlight every line in the interval. + // Note that this crashes successfully on the first non-existing element, + // so you cannot really use '#L1-9999999999 to cause JS overheat. + for (var i = minLine; i <= maxLine; i++) { + lineNode = JX.$('L' + i); + tr = JX.DOM.findAbove(lineNode, 'tr'); + JX.DOM.alterClass(tr, 'phabricator-source-highlight', true); + } + } catch (ex) { + // If the '#L' fragment parser crashed, just move on. + // If we didn't hit an element on the page, just move on. + } + } else { + // in the URI. + // This is from legacy '$123' URIs. try { var anchor = JX.$('phabricator-line-linker-anchor'); JX.DOM.scrollToPosition(0, JX.$V(anchor).y - 60);