Update blame links with selected line, fix bug 1007293#563
Conversation
|
It's all javascript! |
| $this.data('template', $this.attr('href')); | ||
| } | ||
| if (lastNumber) { | ||
| $this.attr('href', $this.data('template').replace(/{{line}}/g, lastNumber)); |
There was a problem hiding this comment.
Does this work? I know that {} are special characters in some regex dialects. Not sure if it is a problem here.
There was a problem hiding this comment.
It did work when I tested it locally. I think curlies are only used in {n} to match n occurrences of the preceding character.
|
This seems like a fragile approach, since if the JS screws up you would get bad urls in your links. Maybe have the thing that generates the HTML fill in |
|
Yes it would! I'll do that and ping you again |
| $this.attr('href', $this.data('template').replace(/{{line}}/g, lastNumber)); | ||
| } else { | ||
| $this.attr('href', $this.data('template').replace(/{{line}}/g, '')); | ||
| } |
There was a problem hiding this comment.
You can be a bit more concise here like this:
$this.attr('href', $this.data('template').replace(/{{line}}/g, lastNumber || ''));
|
@mythmon thanks for the reviews! |
|
|
||
| // Highlight any lines specified by hash in either a direct page load or a history pop. | ||
| $(document).ready(processHash); | ||
| $(document).ready(function() { |
There was a problem hiding this comment.
Why wrap this in a function? The terser original form was equivalent, no?
|
My only reservation is that there's a hanging |
|
Thanks for review! |
3 participants
take 2! This time simplified from #448 (no format bump, mostly a javascript update).
Plugins can put {{line}} in the href of any links, and these will be substituted with the most recently selected line number.