Patchwork [3,of,3] hgweb: add missing semicolons to followlines.js

login
register
mail settings
Submitter Anton Shestakov
Date Nov. 10, 2017, 12:39 p.m.
Message ID <981d7ebd7dc74a88493f.1510317546@neuro>
Download mbox | patch
Permalink /patch/25457/
State Accepted
Headers show

Comments

Anton Shestakov - Nov. 10, 2017, 12:39 p.m.
# HG changeset patch
# User Anton Shestakov <av6@dwimlabs.net>
# Date 1510312446 -28800
#      Fri Nov 10 19:14:06 2017 +0800
# Node ID 981d7ebd7dc74a88493f39982649a52ed09a199b
# Parent  95cb67784e3c0d05e24e387acda45fd1c8aa4653
hgweb: add missing semicolons to followlines.js

Minor stylistic issues caught by jshint.
Denis Laxalde - Nov. 10, 2017, 7:53 p.m.
The series looks good to me, thanks.

Anton Shestakov a écrit :
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1510312446 -28800
> #      Fri Nov 10 19:14:06 2017 +0800
> # Node ID 981d7ebd7dc74a88493f39982649a52ed09a199b
> # Parent  95cb67784e3c0d05e24e387acda45fd1c8aa4653
> hgweb: add missing semicolons to followlines.js
> 
> Minor stylistic issues caught by jshint.
> 
> diff --git a/mercurial/templates/static/followlines.js b/mercurial/templates/static/followlines.js
> --- a/mercurial/templates/static/followlines.js
> +++ b/mercurial/templates/static/followlines.js
> @@ -38,7 +38,7 @@ document.addEventListener('DOMContentLoa
>       // element
>       var selectableElements = Array.prototype.filter.call(
>           sourcelines.children,
> -        function(x) { return x.tagName === selectableTag });
> +        function(x) { return x.tagName === selectableTag; });
>   
>       var btnTitleStart = 'start following lines history from here';
>       var btnTitleEnd = 'terminate line block selection here';
> @@ -62,7 +62,7 @@ document.addEventListener('DOMContentLoa
>       }
>   
>       // extend DOM with CSS class for selection highlight and action buttons
> -    var followlinesButtons = []
> +    var followlinesButtons = [];
>       for (var i = 0; i < selectableElements.length; i++) {
>           selectableElements[i].classList.add('followlines-select');
>           var btn = createButton();
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - Nov. 10, 2017, 10:28 p.m.
On Fri, Nov 10, 2017 at 08:39:06PM +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1510312446 -28800
> #      Fri Nov 10 19:14:06 2017 +0800
> # Node ID 981d7ebd7dc74a88493f39982649a52ed09a199b
> # Parent  95cb67784e3c0d05e24e387acda45fd1c8aa4653
> hgweb: add missing semicolons to followlines.js

Queued, thanks.

>
> Minor stylistic issues caught by jshint.

Should we have some sort of test-jshint.t that can automatically run?
or does it have lots of false positives?

>
> diff --git a/mercurial/templates/static/followlines.js b/mercurial/templates/static/followlines.js
> --- a/mercurial/templates/static/followlines.js
> +++ b/mercurial/templates/static/followlines.js
> @@ -38,7 +38,7 @@ document.addEventListener('DOMContentLoa
>      // element
>      var selectableElements = Array.prototype.filter.call(
>          sourcelines.children,
> -        function(x) { return x.tagName === selectableTag });
> +        function(x) { return x.tagName === selectableTag; });
>
>      var btnTitleStart = 'start following lines history from here';
>      var btnTitleEnd = 'terminate line block selection here';
> @@ -62,7 +62,7 @@ document.addEventListener('DOMContentLoa
>      }
>
>      // extend DOM with CSS class for selection highlight and action buttons
> -    var followlinesButtons = []
> +    var followlinesButtons = [];
>      for (var i = 0; i < selectableElements.length; i++) {
>          selectableElements[i].classList.add('followlines-select');
>          var btn = createButton();
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Anton Shestakov - Nov. 11, 2017, 7:43 a.m.
On Fri, 10 Nov 2017 17:28:33 -0500
Augie Fackler <raf@durin42.com> wrote:

> On Fri, Nov 10, 2017 at 08:39:06PM +0800, Anton Shestakov wrote:
> > # HG changeset patch
> > # User Anton Shestakov <av6@dwimlabs.net>
> > # Date 1510312446 -28800
> > #      Fri Nov 10 19:14:06 2017 +0800
> > # Node ID 981d7ebd7dc74a88493f39982649a52ed09a199b
> > # Parent  95cb67784e3c0d05e24e387acda45fd1c8aa4653
> > hgweb: add missing semicolons to followlines.js
> 
> Queued, thanks.
> 
> >
> > Minor stylistic issues caught by jshint.
> 
> Should we have some sort of test-jshint.t that can automatically run?
> or does it have lots of false positives?

Not a lot, no. And there are plenty of things to tweak:
http://jshint.com/docs/options/

Detecting its availability can be tricky though. It's not as widespread
as even pyflakes, for example there's no node-jshint package in Debian
or Ubuntu. And not everybody is fine using npm install --global (and
messing with their distro's package manager), so my guess is that most
of the time it's installed, it's installed somewhere local, which makes
it impossible to detect and use in a test.

But installing locally, putting a symlink into ~/bin and adding ~/bin
to $PATH does the trick. It's not a very user-friendly way to install a
dependency for just one test file, but it works. Patches sent.

Patch

diff --git a/mercurial/templates/static/followlines.js b/mercurial/templates/static/followlines.js
--- a/mercurial/templates/static/followlines.js
+++ b/mercurial/templates/static/followlines.js
@@ -38,7 +38,7 @@  document.addEventListener('DOMContentLoa
     // element
     var selectableElements = Array.prototype.filter.call(
         sourcelines.children,
-        function(x) { return x.tagName === selectableTag });
+        function(x) { return x.tagName === selectableTag; });
 
     var btnTitleStart = 'start following lines history from here';
     var btnTitleEnd = 'terminate line block selection here';
@@ -62,7 +62,7 @@  document.addEventListener('DOMContentLoa
     }
 
     // extend DOM with CSS class for selection highlight and action buttons
-    var followlinesButtons = []
+    var followlinesButtons = [];
     for (var i = 0; i < selectableElements.length; i++) {
         selectableElements[i].classList.add('followlines-select');
         var btn = createButton();