Page MenuHomePhorge
Files F5481689

D25569.1750111440.diff
No OneTemporary
Actions

D25569.1750111440.diff
View Options

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -466,7 +466,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' => '89bcc30a',
'rsrc/js/core/behavior-linked-container.js' => '74446546',
'rsrc/js/core/behavior-more.js' => '506aa3f4',
'rsrc/js/core/behavior-object-selector.js' => '98ef467f',
@@ -628,7 +628,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' => '89bcc30a',
'javelin-behavior-phabricator-notification-example' => '29819b75',
'javelin-behavior-phabricator-object-selector' => '98ef467f',
'javelin-behavior-phabricator-oncopy' => 'da8f5259',
@@ -986,13 +986,6 @@
'0d2490ce' => array(
'javelin-install',
),
- '0d915ff5' => array(
- 'javelin-behavior',
- 'javelin-stratcom',
- 'javelin-dom',
- 'javelin-history',
- 'javelin-external-editor-link-engine',
- ),
'0eaa33a9' => array(
'javelin-behavior',
'javelin-dom',
@@ -1637,6 +1630,13 @@
'javelin-stratcom',
'javelin-install',
),
+ '89bcc30a' => array(
+ 'javelin-behavior',
+ 'javelin-stratcom',
+ 'javelin-dom',
+ 'javelin-history',
+ 'javelin-external-editor-link-engine',
+ ),
'8ac32fd9' => array(
'javelin-behavior',
'javelin-stratcom',
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,60 @@
}
};
+ /**
+ * Get a valid line number (int) from a string, or scream violently.
+ * Do not accept zero.
+ * Do not accept negative numbers.
+ *
+ * @param {String} vRaw String containing a number, like '1'.
+ * @return {Integer} Integer like 1.
+ */
+ 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].
+ * Note that risky thrash like '#L123-456-789' returns [].
+ * Note that risky thrash like '#Labc-123' returns [].
+ * Note that risky thrash like '#123-abc' returns [].
+ */
+ 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 +205,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 +216,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 +248,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);

File Metadata

Mime Type
text/plain
Expires
Mon, Jun 16, 22:04 (1 h, 9 m ago)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1879464
Default Alt Text
D25569.1750111440.diff (7 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0