>From what I understand false means it will remove extra spaces and true means it will add them if missing. El 21/11/2014 20:39, "Ryan Kaldari" <[email protected]> escribió:
> If you run jsbeautify with space_in_paren: false, does that mean that it > removes spaces or that it ignores whether or not there are spaces? If the > later, I would vote that we do that. > > Kaldari > > On Fri, Nov 21, 2014 at 10:42 AM, Jon Robson <[email protected]> > wrote: > >> I think we can conclude if jscs doesn't complain then it is not an >> established coding convention and we should leave that up to the >> implementer. >> >> Going forward I would suggest in code review we do not nit pick on the >> things that jscs does not complain about, so if Joaquin wants to run >> jsbeautify on his pre-commit hook on new patches and it creates code >> that makes jscs happy he should be allowed to. If there is something >> that you believe should be a coding convention enabling it in jscs >> config if the rule exists or write a new rule for jscs. >> >> If we codify more conventions in jscs then tools like jsbeautify will >> have to be adapted to respect that. >> >> All in favour? >> >> >> On Sat, Nov 22, 2014 at 2:34 AM, Ryan Kaldari <[email protected]> >> wrote: >> > Personally, I like hideOverlay( this.stack[0].overlay ); >> > >> > Having spaces inside parentheses makes sense since parameters are >> usually >> > 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 >> spaces >> > 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 <[email protected]> >> wrote: >> >> >> >> + 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 <[email protected]> >> wrote: >> >> > 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 >> on >> >> > (e.g. >> >> > just change which tasks the "grunt fixup" alias will run). They >> wouldn't >> >> > 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 >> projects >> >> > to >> >> > try out new tools without requiring server-side configuration. I try >> out >> >> > 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 >> won't >> >> > 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 >> having >> >> > 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 >> in >> >> > the >> >> > first place. >> >> > >> >> > And fortunately, jscs actually plans to ship an "autofixer" utility >> that >> >> > will correct violations from within jscs itself. Using the solid >> parser >> >> > and >> >> > tokeniser that jscs is known for. Thus, even the small jscs rule >> config >> >> > we >> >> > do have, won't have to be duplicated. >> >> > >> >> > The jQuery Team also supports this effort for their various >> javascript >> >> > 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 <[email protected]> >> >> > wrote: >> >> > >> >> > Begin forwarded message: >> >> > >> >> > Date: 20 november 2014 21:19:24 CET >> >> > From: Jon Robson <[email protected]> >> >> > To: Bahodir Mansurov <[email protected]> >> >> > Cc: mobile-l <[email protected]> >> >> > 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: >> >> > >> >> > >> https://gist.github.com/jdlrobson/a05ddad00175ebceac68#file-gistfile1-diff-L257 >> >> > >> >> > 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 >> proceed? >> >> > >> >> > >> >> > >> >> > On Wed, Nov 19, 2014 at 4:12 AM, Bahodir Mansurov >> >> > <[email protected]> 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 <[email protected]> >> wrote: >> >> > >> >> > 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 >> adhering >> >> > 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 >> can >> >> > 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 >> sign >> >> > that we should be using functions rather than long if statements >> >> > personally. Again not a blocker. >> >> > >> >> > Actions: >> >> > * Implement https://github.com/beautify-web/js-beautify/issues/424 >> >> > * You guys need to advise me on how we should handle square brackets >> >> > in our codebase in such a way we respect MediaWiki coding conventions >> >> > and come up with a consistent style we are happy with and can adhere >> >> > to. >> >> > * I'll implement https://trello.com/c/0dkx0ldL in some form >> >> > * I'll explore if jscs can support objects defined on new line >> >> > * When the above are done I recommend we turn on jsbeautify for the >> >> > project. >> >> > >> >> > I've setup a tracking card for the above work: >> >> > https://trello.com/c/5btWf2JN/90-snakes-on-a-plane >> >> > and will be looking at these problems today. >> >> > >> >> > [1] https://github.com/beautify-web/js-beautify >> >> > [2] https://github.com/jscs-dev/node-jscs >> >> > >> >> > _______________________________________________ >> >> > Mobile-l mailing list >> >> > [email protected] >> >> > https://lists.wikimedia.org/mailman/listinfo/mobile-l >> >> > >> >> > >> >> > >> >> > _______________________________________________ >> >> > Mobile-l mailing list >> >> > [email protected] >> >> > https://lists.wikimedia.org/mailman/listinfo/mobile-l >> >> > >> >> > >> >> > >> >> >> >> _______________________________________________ >> >> Mobile-l mailing list >> >> [email protected] >> >> https://lists.wikimedia.org/mailman/listinfo/mobile-l >> > >> > >> > > > _______________________________________________ > Mobile-l mailing list > [email protected] > https://lists.wikimedia.org/mailman/listinfo/mobile-l > >
_______________________________________________ Mobile-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mobile-l
