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

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



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 :-))

> +            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.
-- 
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


Reply to: