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
tools: remove ESLint max-len rule #41509
base: master
Are you sure you want to change the base?
Conversation
|
Does this also remove all width restrictions on comments etc? |
I believe so. It does not remove the restriction on markdown file prose text because that is linted with remark-lint rather than ESLint. |
|
I 100% agree that line length limits are a frequent inconvenience and can make code less readable for me personally. And let me preface this by saying that I am definitely no expert when it comes to accessibility, the following is just my understanding of common accessibility guidelines. (If there are accessibility guidelines that are specific to code style, I'd be very interested in reading those.)
@ljharb While I agree with this statement, I believe line length limits serve purposes other than managing complexity. On a standard monitor and with default editor settings, I can open two 80-column text files next to each other without having to scroll in more than one dimension, or one 80-column text file using a much larger font size. Most accessibility guidelines recommend limiting line lengths. For web content, the WCAG require content to be presentable such that the "width is no more than 80 characters or glyphs" and such that "text can be resized without assistive technology up to 200 percent in a way that does not require the user to scroll horizontally to read a line of text on a full-screen window". (Coincidentally, Zoom is also the first section of VS Code's Accessibility docs.) And sure, the WCAG do not require the content to fulfill that property directly, they only require that the content can be presented in such a manner, so it is not uncommon to see the response "enable word wrap in the editor," which works quite well for text with no indentation. However, if editors (and Git and GitHub) were capable of properly re-formatting code to account for user preferences including word wrapping etc., we wouldn't have to worry about formatting in the first place.
@GeoffreyBooth While I personally tend to agree, I don't know if this is true in general. For example, the WAI says:
If the goal is to allow some longer lines here and there, I personally don't think completely disabling this rule is the way to go. We already have some sensible exceptions (and the WCAG do permit such exceptions, e.g., for long URIs) and there are ESLint directives to disable the line length limit, even for entire files if necessary. Aside from adding exceptions and disabling the rule for small parts of the codebase, we could also consider increasing the maximum line length (even though 80 seems to align with most accessibility guidelines) or we could distinguish between soft and hard line length limits. (No objection, just my two cents.) |
|
The airbnb style guide is set to a length of 100, but length checks are ignored on any line with a string, regex, or comment. In many years, I've almost never found a line warned by max-len that was actually an issue. Scrolling is fine for the very rare use case of having two windows tiled - the beginning of the line should be enough context most of the time, and you can enable soft wrapping for every other time. |
I have no idea how much attention the airbnb style guide pays to accessibility guidelines, but either way, this seems to be closer to what I suggested in my previous comment than to disabling the |
|
I’d love to hear some justification for why it’s an accessibility issue at all - tiled windows surely isn’t about accessibility. |
I'm not sure I understand the question. I am literally just quoting existing accessibility guidelines, which say that text should be presentable such that the "width is no more than 80 characters or glyphs" and such that "text can be resized without assistive technology up to 200 percent in a way that does not require the user to scroll horizontally to read a line of text on a full-screen window". |
|
I’m suggesting we don’t take accessibility guidelines on blind faith. I usually can quickly intuit or google the rationale behind a11y guidelines; I really don’t understand why an 80 char limit is an a11y issue. |
|
@ljharb I think what @tniessen is saying is that he's already written out his perspective about why it might be an a11y issue in #41509 (comment) |
|
hmm, fair enough - on a reread tho i still don’t buy it especially given the quantity of content that requires soft-wrapping and the quantity of experiences (including the entire web) that does soft-wrap, but i suppose I’m not in a position to question it. |
|
Perhaps #41536 is a more modest proposal that raises fewer concerns. |
I prefer this proposal. I also see no accessibility concern. Editors have word wrap (and variable font sizes) and we’re not responsible for how GitHub’s UI handles accessibility; and there are plenty of browser extensions to customize that, as well. In my opinion, the upside of more readable code far outweighs the need for some people to use the settings available to them to wrap long lines.
Refs: #41463 (comment)
Refs: #41463 (comment)
The text was updated successfully, but these errors were encountered: