contribute.jquery.org icon indicating copy to clipboard operation
contribute.jquery.org copied to clipboard

Style Guide: Context-changing methods as first method call

Open scottgonzalez opened this issue 10 years ago • 2 comments
trafficstars

From https://github.com/jquery/jquery-ui/pull/1508#issuecomment-86893279:

We can make the rules for chained method calls a bit more lenient, something like "Context changing method calls at the start of the chain are acceptable on the first line.", along with an example, e.g. the filter/attr chain.

Example:

this.tabs.filter( function() {
    return $( this ).attr( "tabIndex" ) === 0;
} )
    .attr( "tabIndex", -1 );

Any arguments against this?

scottgonzalez avatar Mar 27 '15 11:03 scottgonzalez

I dislike it. It seems like something is missing. It seems like someone just forgot to indent it.

My vote goes for either @dmethvin suggestion A or @scottgonzalez suggestion B.

A:

this.tabs.filter( function() {
  return $( this ).attr( "tabIndex" ) === 0;
} ).attr( "tabIndex", -1 );

B:

this.tabs.filter( function() {
        return $( this ).attr( "tabIndex" ) === 0;
    } )
    .attr( "tabIndex", -1 );

PS: The spacy } ) is really awkward. I couldn't get used to it yet :see_no_evil:.

rxaviers avatar Mar 27 '15 14:03 rxaviers

A is definitely not in line with our style guide. B is actually what I expected. See the other thread for @jzaefferer's explanation of why he didn't want the extra indentation.

scottgonzalez avatar Mar 27 '15 14:03 scottgonzalez