Patchwork [5,of,7] hgweb: define locally used variables as actually local in mercurial.js

login
register
mail settings
Submitter Anton Shestakov
Date Nov. 27, 2017, 9:28 a.m.
Message ID <32c7b8d09f80d3006295.1511774892@neuro>
Download mbox | patch
Permalink /patch/25770/
State Accepted
Headers show

Comments

Anton Shestakov - Nov. 27, 2017, 9:28 a.m.
# HG changeset patch
# User Anton Shestakov <av6@dwimlabs.net>
# Date 1511358576 -28800
#      Wed Nov 22 21:49:36 2017 +0800
# Node ID 32c7b8d09f80d30062952d897733b8761fcfe80f
# Parent  a9b86add287cf05dc212fdde9d84d952fbdbf676
# EXP-Topic hgweb-cleanup
hgweb: define locally used variables as actually local in mercurial.js

Variables that are used or assigned without any declaration using var (or let,
or const) are considered global. In many cases this is inadvertent and actually
causes a variable leaking to a broader scope, such as a temporary variable used
inside a loop suddenly being accessible in global scope. (This corresponds to
"undef" option of jshint).

So this patch limits the scope of variables that don't need to be global. There
are a lot of helper variables in Graph.render() used in a loop, I've declared
them all on one line to reduce patch size. "radius" is special because it
wasn't passed to graph.vertex, but was used there (it worked because this
variable leaked to global scope). "window.graph" is created by an inline script
in graph.tmpl so that it can be used in ajaxScrollInit() function, this patch
makes this fact explicit by assigning window.graph to a local variable.

Patch

diff --git a/mercurial/templates/gitweb/graph.tmpl b/mercurial/templates/gitweb/graph.tmpl
--- a/mercurial/templates/gitweb/graph.tmpl
+++ b/mercurial/templates/gitweb/graph.tmpl
@@ -49,7 +49,7 @@  var data = {jsdata|json};
 var graph = new Graph();
 graph.scale({bg_height});
 
-graph.vertex = function(x, y, color, parity, cur) \{
+graph.vertex = function(x, y, radius, color, parity, cur) \{
 	
 	this.ctx.beginPath();
 	color = this.setColor(color, 0.25, 0.75);
diff --git a/mercurial/templates/monoblue/graph.tmpl b/mercurial/templates/monoblue/graph.tmpl
--- a/mercurial/templates/monoblue/graph.tmpl
+++ b/mercurial/templates/monoblue/graph.tmpl
@@ -43,7 +43,7 @@ 
     var graph = new Graph();
     graph.scale({bg_height});
 
-    graph.vertex = function(x, y, color, parity, cur) \{
+    graph.vertex = function(x, y, radius, color, parity, cur) \{
 
         this.ctx.beginPath();
         color = this.setColor(color, 0.25, 0.75);
diff --git a/mercurial/templates/paper/graph.tmpl b/mercurial/templates/paper/graph.tmpl
--- a/mercurial/templates/paper/graph.tmpl
+++ b/mercurial/templates/paper/graph.tmpl
@@ -62,7 +62,7 @@  var data = {jsdata|json};
 var graph = new Graph();
 graph.scale({bg_height});
 
-graph.vertex = function(x, y, color, parity, cur) \{
+graph.vertex = function(x, y, radius, color, parity, cur) \{
 	
 	this.ctx.beginPath();
 	color = this.setColor(color, 0.25, 0.75);
diff --git a/mercurial/templates/spartan/graph.tmpl b/mercurial/templates/spartan/graph.tmpl
--- a/mercurial/templates/spartan/graph.tmpl
+++ b/mercurial/templates/spartan/graph.tmpl
@@ -43,7 +43,7 @@  var data = {jsdata|json};
 var graph = new Graph();
 graph.scale({bg_height});
 
-graph.vertex = function(x, y, color, parity, cur) \{
+graph.vertex = function(x, y, radius, color, parity, cur) \{
 	
 	this.ctx.beginPath();
 	color = this.setColor(color, 0.25, 0.75);
diff --git a/mercurial/templates/static/mercurial.js b/mercurial/templates/static/mercurial.js
--- a/mercurial/templates/static/mercurial.js
+++ b/mercurial/templates/static/mercurial.js
@@ -94,6 +94,7 @@  function Graph() {
 
 		var backgrounds = '';
 		var nodedata = '';
+		var line, start, end, color, x, y, x0, y0, x1, y1, column, radius;
 
 		for (var i = 0; i < data.length; i++) {
 
@@ -146,7 +147,7 @@  function Graph() {
 			radius = this.box_size / 8;
 			x = this.cell[0] + this.box_size * column + this.box_size / 2;
 			y = this.bg[1] - this.bg_height / 2;
-			var add = this.vertex(x, y, color, parity, cur);
+			var add = this.vertex(x, y, radius, color, parity, cur);
 			backgrounds += add[0];
 			nodedata += add[1];
 
@@ -297,7 +298,7 @@  function format(str, replacements) {
 }
 
 function makeRequest(url, method, onstart, onsuccess, onerror, oncomplete) {
-    xhr = new XMLHttpRequest();
+    var xhr = new XMLHttpRequest();
     xhr.onreadystatechange = function() {
         if (xhr.readyState === 4) {
             try {
@@ -344,8 +345,8 @@  function ajaxScrollInit(urlFormat,
                         containerSelector,
                         messageFormat,
                         mode) {
-    updateInitiated = false;
-    container = document.querySelector(containerSelector);
+    var updateInitiated = false;
+    var container = document.querySelector(containerSelector);
 
     function scrollHandler() {
         if (updateInitiated) {
@@ -382,6 +383,7 @@  function ajaxScrollInit(urlFormat,
                 },
                 function onsuccess(htmlText) {
                     if (mode === 'graph') {
+                        var graph = window.graph;
                         var sizes = htmlText.match(/^\s*<canvas id="graph" width="(\d+)" height="(\d+)"><\/canvas>$/m);
                         var addWidth = sizes[1];
                         var addHeight = sizes[2];
@@ -468,7 +470,7 @@  function renderDiffOptsForm() {
             continue;
         }
 
-        currentValue = form.getAttribute("data-" + key);
+        var currentValue = form.getAttribute("data-" + key);
         checkbox.checked = currentValue !== "0";
 
         // ignorews implies ignorewsamount and ignorewseol.
diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t
--- a/tests/test-hgweb-commands.t
+++ b/tests/test-hgweb-commands.t
@@ -1792,7 +1792,7 @@  Overviews
   var graph = new Graph();
   graph.scale(39);
   
-  graph.vertex = function(x, y, color, parity, cur) {
+  graph.vertex = function(x, y, radius, color, parity, cur) {
   	
   	this.ctx.beginPath();
   	color = this.setColor(color, 0.25, 0.75);
diff --git a/tests/test-hgweb-empty.t b/tests/test-hgweb-empty.t
--- a/tests/test-hgweb-empty.t
+++ b/tests/test-hgweb-empty.t
@@ -306,7 +306,7 @@  Some tests for hgweb in an empty reposit
   var graph = new Graph();
   graph.scale(39);
   
-  graph.vertex = function(x, y, color, parity, cur) {
+  graph.vertex = function(x, y, radius, color, parity, cur) {
   	
   	this.ctx.beginPath();
   	color = this.setColor(color, 0.25, 0.75);