Re: [WikimediaMobile] Using jsbeautify in MobileFrontend

Personally, I like hideOverlay( this.stack[0].overlay ); Having spaces inside parentheses makes sense since parameters are

words or word-like and it's easier to read words when they have spaces around them. Keys inside brackets are often just numbers, which I don't think really benefit from having spaces. That said, I realize this is inconsistent and I would be OK with using spaces for both. Not using

at all makes it harder to scan code for particular variables, etc, IMO. Kaldari On Fri, Nov 21, 2014 at 8:56 AM, Jon Robson <jrobson(a)wikimedia.org>

> > + mobile-l > > This is all useful feedback. > That would be ideal if jscs had this capability but an interim > solution would be good in the meantime. > > I think it's important to codify our coding conventions across the > project. It will make it much easier for newbies to write code without > having to worry about coding style. > > > On Fri, Nov 21, 2014 at 11:19 PM, Krinkle <krinklemail(a)gmail.com>

> > Hi Jon, > > > > Just wanted to quickly share my ideas on code formatting. > > > > First off, as long as there are no side effects (e.g. normalising too > > much), > > any tool will do in the mean-while and it's trivial to switch later

> > (e.g. > > just change which tasks the "grunt fixup" alias will run). They

> > be > > run as part of "grunt test". Instead it's a convenience tool for > > developers > > to easily reformat code locally before submission (e.g. after jscs > > pointed > > out one or more errors in their local grunt run). This is among the > > reasons > > why using a local Grunt is so convenient. It enables individual

> > to > > try out new tools without requiring server-side configuration. I try

> > new > > tasks all the time in oojs-core and VisualEditor. Some stick, some > > don't. > > > > Having said that, I've used js-beautifier in the past and worry it

> > work well for us. > > > > esformatter[1] on the other hand seems to have a more stable > > implementation > > and general approach. > > > > However, any tool other than jscs will come with the down side of

> > to > > declare your style guide, again. Which will amount to tedious > > duplication > > and loads of edge cases where the tools declare the style using > > different > > logical rules and will never match. > > > > jscs ships with a wikimedia preset that saves lots of configuration

> > the > > first place. > > > > And fortunately, jscs actually plans to ship an "autofixer" utility

> > will correct violations from within jscs itself. Using the solid

> > and > > tokeniser that jscs is known for. Thus, even the small jscs rule

> > we > > do have, won't have to be duplicated. > > > > The jQuery Team also supports this effort for their various

> > projects (both as style checker, as we do already, and as code > > formatter). > > So that's a major player we'll have on our side when it comes to > > continued > > support for the tool. > > > > https://github.com/jscs-dev/node-jscs/issues/516 > > > > Supported by Mike Sherov (mikesherov) and Oleg Gaidarenko (markelog), > > from > > the jQuery Team (who are also jscs collaborators). And Marat Dulin > > (mdevils), founder of jscs. > > > > Best, > > > > — Krinkle > > > > [1] https://github.com/millermedeiros/esformatter > > > > On 20 Nov 2014, at 23:57, Derk-Jan Hartman <d.j.hartman(a)gmail.com> > > wrote: > > > > Begin forwarded message: > > > > Date: 20 november 2014 21:19:24 CET > > From: Jon Robson <jrobson(a)wikimedia.org> > > To: Bahodir Mansurov <bmansurov(a)wikimedia.org> > > Cc: mobile-l <mobile-l(a)lists.wikimedia.org> > > Subject: Re: [WikimediaMobile] Using jsbeautify in MobileFrontend > > > > Follow up > > If I run `make jsbeautify` now > > then https://gist.github.com/jdlrobson/a05ddad00175ebceac68 is the > > result. > > > > Outstanding actions: > > * Need input on rest of the team about which of delete > > this.cache[title]; or delete this.cache[ title ]; is the preferred > > standard. > > * You'll notice jsbeautify and inlne comments need to be ironed out. > > For example: > > > >

> > > > Apart from the above 2 jsbeautify makes some adjustments to our > > whitspace which I guess we'll just have to live with. > > > > Please can everyone else let me know how they think we should

> > > > > > > > On Wed, Nov 19, 2014 at 4:12 AM, Bahodir Mansurov > > <bmansurov(a)wikimedia.org> wrote: > > > > As for # Rule 4, it makes sense to add spaces inside square brackets. > > The > > reasoning is the same as why we add spaces inside parenthesis. > > > > On Nov 18, 2014, at 12:28 PM, Jon Robson <jrobson(a)wikimedia.org>

> > > > I explored running jsbeautify [1] on the MobileFrontend codebase and > > looked at how the output differs from the current styling. It > > introduces various rules that MobileFrontend is currently not

> > too. MobileFrontend already uses jscs [2] so we want to make sure the > > outputs of both are compatible. Here is my report on that with the > > recommendation that we should use it. > > > > #Rule 1: object properties defined on a single line. > > e.g. > > { > > foo: 2, > > bar: 3 > > } > > NOT { foo: 2, bar: 3 } > > > > I think this would be a good idea to adopt. I will explore if jscs

> > enforce this. > > > > # Rule 2: variables that are initialised must be followed by a new > > line (although I noted a few odd cases e.g. in Page.js after a "?:" > > expression and /MobileWebClickTracking.js > > e.g. > > var Foo, bar = $( 'div' ), > > Baz, > > Bar; > > > > not: > > var Foo, bar = $( 'div' ), Baz, Bar; > > > > This will be fixed if I implement https://trello.com/c/0dkx0ldL > > > > # Rule 3: All chained events should be indented > > e.g. > > foo() > > .bar(); > > > > not > > foo(). > > bar(); > > > > Seems like a no brainer. One that happens naturally most of the time. > > > > # Rule 4: Spacing in object parameters > > e.g. > > foo[ 1 ] > > [ 1, 2, 3, 4 ] > > > > not: > > foo[1] > > [1, 2, 3, 4] > > > > This is different to MediaWiki coding conventions but I can implement > > https://github.com/beautify-web/js-beautify/issues/424 to give us > > this. > > We seem a bit inconsistent ourselves with this convention - let me > > know how you think this rule should work in our codebase... > > > > # Rule 5: New line after variable declarations > > > > var x, y, z; > > > > z(); > > > > not: > > var x, y, z; > > z(); > > > > Also: > > function foo() {} > > > > function bar() {} > > > > not: > > function foo() {} > > function bar() {} > > > > Seems like a no brainer and shouldn't introduce any major issues with > > code review. > > > > # Rule 6: Comments must respect the current indentation level > > > > if () { > > ... > > // If i is 5 we do something special > > } else if ( i === 5 ){ > > > > } > > becomes > > if () { > > ... > > // If i is 5 we do something special > > } else if ( i === 5 ){ > > > > } > > We'll have to think about what to do with these comments but I don't > > think this should be a blocker to using jsbeautify. It will only pop > > up occasionally. > > > > # Rule 7: On long if statements the curly brace must be indented. > > And the first condition should be on the same line > > > > if ( enableToc || ... > > .... > > && baz ) { > > > > > > not: > > if ( > > enableToc || ... > > .... > > && baz ) { > > > > Again I'm not sure if this will happen too often. This to me is a