Hello again, Jason.
To JavaScript hackers lurking here: your comments on this patch are most
welcome!
On Sat, Nov 01, 2014 at 11:37:56AM -0400, Jason Pleau wrote:
> I have attached a patch that solves this bug.
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.
> +<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).
Cheers.
--
Stefano Zacchiroli . . . . . . . zack@upsilon.cc . . . . o . . . o . o
Maître de conférences . . . . . http://upsilon.cc/zack . . . o . . . o o
Former Debian Project Leader . . @zack on identi.ca . . o o o . . . o .
« the first rule of tautology club is the first rule of tautology club »
Attachment:
signature.asc
Description: Digital signature