Hello Stefano,
The attached patch implements your previous suggestions :
On 06/11/14 02:22 PM, Stefano Zacchiroli wrote:
> Hello again, Jason.
>
> To JavaScript hackers lurking here: your comments on this patch are most
> welcome!
>
> Thanks a lot for this patch. It works like a charm on my local
> deployment. As I'm no JavaScript guru, I could use extra eyeballs to
> review it, and I do have some questions for you (see below), but I see
> no reason why this couldn't be integrated.
>
>> - <a id="L{{ i }}" href="#L{{ i }}">{{ i }}</a><br />
>> + <a id="L{{ i }}" href="#L{{ i }}" class="linenumber" data-line="{{
>> i }}">{{ i }}</a><br />
>
> Is it really needed to add class="linenumber" to all <a> elements?
>
> It seems that the JavaScript only uses that class as a selector. So
> can't one instead rely on the id="sourcelinenumbers" that is already on
> the parent <pre> element, and consider all its <a> children? That would
> avoid bloat in the generated HTML, reduce load time, etc.
Indeed, it was not needed to add a new class. Using the <pre> element's
#id is a better choice.
>
>> +<script type="text/javascript">
> [...]
>> +</script>
>
> I'd like to move this JavaScript snippet to a separate .js file, rather
> than re-shipping it every time a file is rendered. Can you change your
> patch to do so?
>
> I suggest to name it something like
> DEBSOURCES_ROOT/debsources/app/static/javascript/debsources.js (so that
> we can use it in the future for other JavaScript-related needs). It will
> then be accessible from the HTML as "/static/javascript/debsources.js"
> (note: *not* as /javascript/debsources.js).
>
I have added a new file:
DEBSOURCES_ROOT/debsources/app/static/javascript/debsources.js
It is only included in the source_file.html template, there's no need
(yet) to force the download of this javascript file in other templates.
The code is now wrapped in an an anonymous function, to not pollute the
global namespace if this javascript file is ever loaded in other pages.
Thanks
--
Jason Pleau
From 8132506a6d178db416f852161e279d367529d8f7 Mon Sep 17 00:00:00 2001
From: Jason Pleau <ja...@jpleau.ca>
Date: Sat, 1 Nov 2014 10:44:24 -0400
Subject: [PATCH] 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.
---
debsources/app/static/javascript/debsources.js | 116 +++++++++++++++++++++
debsources/app/templates/source_file.html | 1 +
debsources/app/templates/source_file_code.inc.html | 7 +-
3 files changed, 119 insertions(+), 5 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..20b81d8
--- /dev/null
+++ b/debsources/app/static/javascript/debsources.js
@@ -0,0 +1,116 @@
+/* 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.getAttribute('data-line'));
+ } else {
+ var first_line = parseInt(last_clicked.getAttribute('data-line'));
+ var second_line = parseInt(callerElement.getAttribute('data-line'));
+
+ 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..472ac6b 100644
--- a/debsources/app/templates/source_file_code.inc.html
+++ b/debsources/app/templates/source_file_code.inc.html
@@ -19,17 +19,14 @@
<tr>
<td>
<pre id="sourceslinenumbers">{% for i in range(1, nlines+1) -%}
- <a id="L{{ i }}" href="#L{{ i }}">{{ i }}</a><br />
+ <a id="L{{ i }}" href="#L{{ i }}" data-line="{{ i }}">{{ i }}</a><br />
{%- endfor %}</pre>
</td>
<td>
<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
