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 <ja...@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 <z...@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 <ja...@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