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

Limit Maximum Length of Line #28

Closed
mightyiam opened this issue Feb 5, 2015 · 20 comments
Closed

Limit Maximum Length of Line #28

mightyiam opened this issue Feb 5, 2015 · 20 comments

Comments

@mightyiam
Copy link
Member

This is awesome.

Any thoughts on row width?

@mightyiam mightyiam changed the title Column width Row width Feb 5, 2015
@yoshuawuyts
Copy link
Contributor

@mightyiam I'm not entirely sure what you mean, can you be a bit more specific with your question? thanks!

@mightyiam mightyiam changed the title Row width Row length Feb 6, 2015
@mightyiam
Copy link
Member Author

Perhaps I mean row length.

JSHint has the maxlen option. I know we're using ESLint.

Good. ESLint also has max-len.

For our enlightenment:

So it seems that 80 is the most popular and for what it is worth, I'd gladly accept 80 as the standard.

However, I'd love to be enlightened more if anyone has compelling advice in favor of a different value.

@mightyiam
Copy link
Member Author

JSCS has maximumLineLength.

@feross
Copy link
Member

feross commented Feb 7, 2015

In general, I try not to make lines longer than 100 characters, but it's not a hard-and-fast rule. I'm -1 on adding this rule.

If you have a long string (example), especially in a test file, you don't gain anything by splitting it into multiple lines. It doesn't protect you from bugs, or even improve readability.

@feross
Copy link
Member

feross commented Feb 7, 2015

Also, this new rule would require a major version bump. Lots of the modules listed here have lines longer than 100.

A quick note on philosophy: The idea is for standard to be "done" at some point in time (very soon). I want standard to be well-understood and reliable, with minimal bike-shedding and no new rules so we can all rely on it as a baseline in our modules :)

@mightyiam
Copy link
Member Author

I understand that we want backwards compatibility and I understand that we want it to be done.

I do not agree that long strings are a good reason not to have a Standard row length limit.

For one, because ES6 has multi–line strings!

And another reason is that the size of the windows on people's desktops are a big (huge?) important part of how people develop and if the row length is not standardized when we have a chance now then that is just a shame, isn't it?

Standard row length will allow everyone to never again resize their terminal or GUI editor windows, moving from working on one project to a different project! That seems huge to me.

On an 1920 pixels wide display (already seems minimal standard for desktop displays and gradually becoming minimal standard for laptops, as well), this is what it looks like for me:
thirds-with-ubuntu-side-panel

Notice that 100 rows don't fit. What do we see? We see the Ubuntu Unity side panel and a gnome-terminal window.

The terminal window is with a font of size 9 (which is rather small, I would say). In it there is a byobu tmux session, dividing the the terminal into 67% & 33%. The former 67% part has a Vim running, divided in the middle.

I use the 33% right part for on–change test running (standard will be one of them once #21 is fixed).

Vim has some plugins that take up some row length, like line numbers, change markers and syntax checking.

Overall, this seems to me like a reasonable—and I would image common—setup.

Addressing the font size again; opening up a guest Ubuntu session, turns out that the default is 12. So my 9 is pretty small, I would say again.

The caret in the screenshot, if you can spot it, is at column 97, which seems to be the last.

With the default font size 12, the last column of this same setup is 72.

So, I would say that 80 would seem to me the reasonable choice, since it is already a very popular choice, not only amongst JS developers, but amongst other languages, as well.

Concerning existing very long lines—they can quite easily put in a comment rule and keep their long lines.

As for large code bases, which have a big amount of > 80 length rows, they can use a global rule in an rcfile.

I've convinced myself, anyway.

@feross feross changed the title Row length Limit Maximum Length of Line Feb 9, 2015
@dcousens
Copy link
Member

In general, I try not to make lines longer than 100 characters, but it's not a hard-and-fast rule. I'm -1 on adding this rule.

I'm also -1 on this rule, I personally soft-limit to 120 characters, but am happy with large constants wrapping around if necessary.

@jprichardson
Copy link
Member

I'm also against this.

On Tue, Feb 10, 2015 at 8:09 PM, Daniel Cousens notifications@github.com
wrote:

In general, I try not to make lines longer than 100 characters, but it's
not a hard-and-fast rule. I'm -1 on adding this rule.

I'm also -1 on this rule, I personally limit to 120 characters, but am
happy with large constants wrapping around if necessary.


Reply to this email directly or view it on GitHub
#28 (comment).

Simple & Secure Bitcoin Wallet: https://www.coinbolt.com
Bitcoin / JavaScript: http://cryptocoinjs.com
Follow JP Richardson on Twitter: https://twitter.com/jprichardson

@feross
Copy link
Member

feross commented Feb 11, 2015

Thanks for the feedback, everyone. I think we can safely close this issue now because there's not enough support.

@feross feross closed this as completed Feb 11, 2015
@mightyiam
Copy link
Member Author

😠 😊 😞

@nilsboy
Copy link

nilsboy commented Oct 14, 2015

+1 for 80 column line length.

@wilmoore
Copy link

I realize this is closed, but FWIW, I'm -1 on this too. I agree 100% with the following:

In general, I try not to make lines longer than 100 characters, but it's not a hard-and-fast rule. I'm -1 on adding this rule.

@rstacruz
Copy link
Member

FWIW: even if i'm strict about 80-char limits on my projects, I'm also -1 on this. I can see this as a barrier to adoption because there really are special cases where you lines past 80 chars.

@nilsboy
Copy link

nilsboy commented Oct 15, 2015

A good formatter in my eyes should be able to always generate the same format no matter what happend to the file in the meantime.
If I work on someone elses project I want to be able to locally work with my own formatting. Before I upload I want to be able to use the projects formatting to recover the original format.
If I locally (e.g.) use my own line length and standard does not touch it when I use it to format my changes before upload - this workflow is not possible.
A good formatter should be able to even recover the original format from an uglification to a degree.

@dcousens
Copy link
Member

@mightyiam that is an interesting post, but, I almost always edit full screen, so the 80 char limit is entirely pointless for me.

@rstacruz
Copy link
Member

@nilsboy I assume you're thinking of standard-format (standard -F). You can also argue that a good formatter will not introduce any ambiguous changes to your code, and there's (likely) no satisfactory way to programmatically rewrite code to a max line length.

@rogeriopvl
Copy link

I realise this is closed, but this feature should be added... or configurable through package.json maybe? Long lines affect code readability.

@mightyiam
Copy link
Member Author

@rogeriopvl standard uses eslint under the hood. Just use max-len.

@rogeriopvl
Copy link

@mightyiam oh! awesome! didn't know that! Thanks.

@rhys-vdw
Copy link

Sounds like nobody cares (or cared even), but it is possible to set a regex checker so that lines with long string constants are excluded from the ESLint line length rule. Nonetheless I agree that it's not an appropriate rule. If anybody's interested this is the (embarrassingly long) regex I use:

var ignoreLongStrings = "\"(?:[^\"\\\\]|\\\\.){60,}\"|'(?:[^'\\\\]|\\\\.){60,}'|`(?:[^`\\\\]|\\\\.){60,}`"
module.exports = {
  rules: {
    'max-len': [warning, 80, 2, { ignoreComments: true, ignorePattern: ignoreLongStrings }]
  }
}

blakeembrey added a commit to blakeembrey/tslint-config-standard that referenced this issue Jan 19, 2017
@lock lock bot locked as resolved and limited conversation to collaborators May 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

10 participants