Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style Guide: Context-changing methods as first method call #107

Open
scottgonzalez opened this issue Mar 27, 2015 · 2 comments
Open

Style Guide: Context-changing methods as first method call #107

scottgonzalez opened this issue Mar 27, 2015 · 2 comments

Comments

@scottgonzalez
Copy link
Member

From jquery/jquery-ui#1508 (comment):

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?

@rxaviers
Copy link
Member

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 🙈.

@scottgonzalez
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants