JS Style Guide: 100 characters for line length too short?

[jquery]

JS Style Guide: 100 characters for line length too short?

[dmethvin]

A lot of that is due to indentation? If this were fresh code we could look at refactoring or other limits such as cyclomatic complexity that would help, but since we're trying to impose this ex-post-facto it might be easier to just raise the limit for now.

[mikesherov]

maxindent from jshint would help alot. Simply changing some nested ifs to guard clauses would help to, but I'm wondering about code readibility and overall size. It seems like 100 characters may actually hurt readibility here.

[dmethvin]

I agree the reformatting does make it look ugly, often due to the high indent levels. This could be fixed with a lot of yak shaving to clean up the intent levels and refactor code with things like early-out returns but it would be a shame to make that a blocker for getting this done.

[scottgonzalez]

120 seems excessively wide to me, but I'd be fine allowing projects to temporarily disable the 100 character check. Projects with a high number of long lines can take the time to adjust them before re-enabling the check. Reducing the indentation levels and staying under 100 characters will improve the readability more than just bumping the limit to 120.

[mikesherov]

@scottgonzalez sounds fine to me. @markelog please note this for core.

[markelog]

please note this for core.

In what way? In jquery/jquery#1590 most changes are done for comments, others, for things like:

-           if ( typeof dataTypeOrTransport === "string" && !seekingTransport && !inspected[ dataTypeOrTransport ] ) {
 +
 +          if (
 +              typeof dataTypeOrTransport === "string" &&
 +              !seekingTransport &&
 +              !inspected[ dataTypeOrTransport ]
 +          ) {

Which for me is cleaner and easier to read, isn't that why we impose 100 chars limit in the first place? Not sure if guards would helps with things like that.

Big changes are scary, right, but this is only code-style, we could do that temporary thing, but i'm not sure why, i already done everything necessary to make that permanent.

If, however, project wanted to do this 120 (or something) -> 100 limit change in iterative way, developer can override that rule, simple as that.

So to make this clear, what are wanted to do here? And what are we wanted to do in Core?

[scottgonzalez]

That conditional format isn't what's in our style guide. The correct format is:

if ( typeof dataTypeOrTransport === "string" &&
        !seekingTransport &&
        !inspected[ dataTypeOrTransport ] ) {
    // body goes here
}

We can certainly discuss changing this, but based on our current style guide, what you've used is incorrect.

[mgol]

I prefer the current format that Scott described. Moving the opening brace
to the left seems inconsistent.