[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#761103: debsources: highlight #Lxxx lines by default



Hello Stefano,

On 07/11/14 12:24 PM, Stefano Zacchiroli wrote:
> On Thu, Nov 06, 2014 at 08:09:15PM -0500, Jason Pleau wrote:
>> The attached patch implements your previous suggestions :
> 
> Awesome, thanks! Last comments:
> 
> 1) I've added to the new .js file a copyright header pointing to
>    you. Can you confirm you're OK with contributing your code under
>    AGPLv3, as the rest of Debsources?
> 
>    You can find attached a version of your patch that includes the
>    copyright header (and fixes some whitespace issues that make Git
>    cry :-))

I'm OK with my code being under this license ! I'll use this copyright
header if I ever need to create new files in the Debsources project.

> 
>> +            change_hash_without_scroll(callerElement, "L" + callerElement.getAttribute('data-line'));
>> +        } else {
>> +            var first_line = parseInt(last_clicked.getAttribute('data-line'));
>> +            var second_line = parseInt(callerElement.getAttribute('data-line'));
> [...]
>> +        <a id="L{{ i }}" href="#L{{ i }}" data-line="{{ i }}">{{ i }}</a><br />
> 
> 2) In the same anti-bloat vein of my previous comments, do we really
>    need to another attribute here, given that its content is precisely
>    the same of the text child of <a>? Can't you just use the data
>    content of that text node?
> 
>    (Yes, I understand that in the future the <a> node might contain more
>    complex markup, but there will always be a text leaf in the DOM tree
>    that we can use; if not directly a DOM method that return the textual
>    value of the whole sub-tree.)
> 
> What do you think?
> Cheers.
>

I usually prefer to not rely on elements' innerHTML/innerText content
(that's just a personal style). But I also always only work in LAN
environments, where speed isn't really an issue. I agree that if we're
browsing a 10k+ line files, those extra bytes can add up quickly.

Please see the attached patch for the fixed version.

Thanks for the feedback !

Jason

>From c1a218f019368d6ea6cedab2d813073ade685a32 Mon Sep 17 00:00:00 2001
From: Jason Pleau <jason@jpleau.ca>
Date: Sat, 1 Nov 2014 10:44:24 -0400
Subject: [PATCH] web app: automatically highlight #Lxx lines

Add automatic line numbers from #Lxx location.hash

It supports #L50 and #L50-L150 (The latter will highlight from line 50 to line
150).

It also changes the hash if we click on a line, and if we click on a second
line holding the SHIFT key, it will highlight the whole range, and update the
hash as well.

Closes: #761103

Signed-off-by: Stefano Zacchiroli <zack@upsilon.cc>
---
 debsources/app/static/javascript/debsources.js     | 137 +++++++++++++++++++++
 debsources/app/templates/source_file.html          |   1 +
 debsources/app/templates/source_file_code.inc.html |   5 +-
 3 files changed, 139 insertions(+), 4 deletions(-)
 create mode 100644 debsources/app/static/javascript/debsources.js

diff --git a/debsources/app/static/javascript/debsources.js b/debsources/app/static/javascript/debsources.js
new file mode 100644
index 0000000..38e3647
--- /dev/null
+++ b/debsources/app/static/javascript/debsources.js
@@ -0,0 +1,137 @@
+/* Copyright (C) 2014  Jason Pleau <jason@jpleau.ca>
+ *
+ * This file is part of Debsources.
+ *
+ * Debsources is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU Affero General Public License as published by the Free
+ * Software Foundation, either version 3 of the License, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Highlight line numbers according to data received in the anchor
+ *
+ * Example: file.cpp#L50-L275 will highlight lines 50 to 275.
+ *
+ * There's also support to select one line or a range of lines (by clicking on
+ * a line or shift-clicking on a range of lines). The URL will be updated with
+ * the selection.
+ *
+ */
+
+(function() {
+    function highlight_lines(start, end) {
+        // First, remove the highlight class from elements that might already have it
+        var elements = document.querySelectorAll("span.highlight");
+        for (i = 0; i < elements.length; ++i) {
+            var element = elements[i];
+            element.className = element.className.replace(/\bhighlight\b/, '');
+        }
+
+        // Then, add the highlight class to elements that contain the lines we want to highlight
+        for (i = start; i <= end; ++i) {
+            var element = document.getElementById("line" + i);
+            element.className = element.className + " highlight ";
+        }
+    }
+
+    var hash_changed = function(event, scroll) {
+
+        event = typeof event !== 'undefined' ? event: null;
+        scroll = typeof scroll !== 'undefined' ? scroll: false;
+
+        // Will match strings like #L15 and #L15-L20
+        var regex = /#L(\d+)(-L(\d+))*$/;
+
+        var match = regex.exec(window.location.hash);
+        if (match != null) {
+            var first_line = second_line = null;
+            first_line = parseInt(match[1]);
+
+            if (typeof match[3] !== 'undefined' && match[3].length > 0) {
+                second_line = parseInt(match[3]);
+            } else {
+                second_line = first_line;
+            }
+
+            // If we get something like #L20-L15, just swap the two line numbers so the loop will work
+            if (second_line < first_line) {
+                var tmp = first_line;
+                first_line = second_line;
+                second_line = tmp;
+            }
+
+            highlight_lines(first_line, second_line);
+
+            if (scroll) {
+                window.scroll(0, document.getElementById("L"+first_line).offsetTop);
+            }
+        }
+    }
+
+
+    function change_hash_without_scroll(element, hash) {
+        // This is necessary because when changing window.location.hash, the window will
+        // scroll to the element's id if it matches the hash
+        var id = element.id;
+        element.id = id+'-tmpNoScroll';
+        window.location.hash = hash;
+        element.id = id;
+    }
+
+    var last_clicked;
+    var line_click_handler = function(event) {
+        if (event.preventDefault) {
+            event.preventDefault();
+        } else {
+            event.returnValue = false;
+        }
+
+        var callerElement = event.target || event.srcElement;
+
+        if (!event.shiftKey || !last_clicked) {
+            last_clicked = callerElement;
+            change_hash_without_scroll(callerElement, "L" + callerElement.innerText);
+        } else {
+            var first_line = parseInt(last_clicked.innerText);
+            var second_line = parseInt(callerElement.innerText);
+
+            if (second_line < first_line) {
+                var tmp = first_line;
+                first_line = second_line;
+                second_line = tmp;
+            }
+
+            change_hash_without_scroll(callerElement, "L" + first_line + "-L" + second_line);
+        }
+    };
+
+    var window_load_sourcecode = function(event) {
+        var line_numbers = document.querySelectorAll("#sourceslinenumbers a");
+        for (i = 0; i < line_numbers.length; ++i) {
+            var line_number_element = line_numbers[i];
+            if (line_number_element.addEventListener) {
+                line_number_element.addEventListener('click', line_click_handler, false);
+            } else {
+                line_number_element.attachEvent('onclick',  line_click_handler);
+            }
+        }
+        hash_changed(null, true);
+    };
+
+    if (window.addEventListener) {
+        window.addEventListener('load', window_load_sourcecode, false);
+    } else {
+        window.attachEvent('onload', window_load_sourcecode);
+    }
+
+    window.onhashchange = hash_changed;
+})();
diff --git a/debsources/app/templates/source_file.html b/debsources/app/templates/source_file.html
index 9e7af5c..3e098a5 100644
--- a/debsources/app/templates/source_file.html
+++ b/debsources/app/templates/source_file.html
@@ -11,6 +11,7 @@
   <link rel="stylesheet"
         href="{{ config.HIGHLIGHT_JS_FOLDER }}/styles/{{ config.HIGHLIGHT_STYLE }}.css">
   <script src="{{ config.HIGHLIGHT_JS_FOLDER }}/highlight.min.js"></script>
+  <script src="{{ url_for('static', filename='javascript/debsources.js') }}"></script>
   <link rel="stylesheet" type="text/css"
         href="{{ url_for('static', filename='css/source_file.css') }}" />
 
diff --git a/debsources/app/templates/source_file_code.inc.html b/debsources/app/templates/source_file_code.inc.html
index 9ea5a88..6394f23 100644
--- a/debsources/app/templates/source_file_code.inc.html
+++ b/debsources/app/templates/source_file_code.inc.html
@@ -26,10 +26,7 @@
       <pre><code id="sourcecode" class="{% if file_language -%}
 					{{ file_language }}{% else %}no-highlight
 					{%- endif %}">{% for (line, highlight) in code -%}
-          {% if highlight -%}
-          <span class="highlight">{{ line }}</span>{% else -%}
-          {{ line }}
-          {%- endif %}
+            <span id="line{{ loop.index }}" class="codeline {% if highlight -%} highlight   {%- endif %}">{{ line }}</span>
           {%- endfor %}</code></pre>
     </td>
     {% if msg -%}
-- 
2.1.3


Reply to: