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

Line-length-limit bikeshed #6041

Closed
waddlesplash opened this issue May 13, 2015 · 5 comments
Closed

Line-length-limit bikeshed #6041

waddlesplash opened this issue May 13, 2015 · 5 comments

Comments

@waddlesplash
Copy link

@waddlesplash waddlesplash commented May 13, 2015

from IRC:

<@mbrubeck> pcwalton: That's interesting - I thought `mach test-tidy` would reject >100char lines.
<@pcwalton> it doesn't, because of the codegen in script IIRC
<@mbrubeck> Oh, it only rejects at 160.
<@pcwalton> it only rejects *really* long ones
<@mbrubeck> generated bindings are excluded from tidy completely.
<@pcwalton> oh, hmm
<@pcwalton> I wonder why we didn't go for 100 across the board then
<@mbrubeck> We have 1620 lines to fix before we can enforce a 100-char limit. :/

CC @mbrubeck @pcwalton @jdm @Ms2ger -- hopefully I didn't miss anyone...

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented May 14, 2015

I limited to 160 because that was about the smallest number that didn't require me fixing a million cases; I'd definitely like to reduce that to 100 or even 80.

@larsbergstrom
Copy link
Contributor

@larsbergstrom larsbergstrom commented May 14, 2015

I'm a little worried about trying to get to even 100 with Rust. There's a lot of rightward drift from nested match, closures, etc. I suspect that if we try to go to 80, when we find that's very hard for lots of code we'll start contorting the code (e.g., lifting things into functions just so they get left-dented) and switching from 4 spaces to 2 just to meet a standard that is mainly motivated by small, narrow screens/terminals/or in-browser UIs trying to do side-by-side diffs with a ton of extra HTML doogadgetry inbetween.

I'd probably want to sit down with the 1620 lines and make sure that truncating them is just a matter of indenting function calls or parameter lists across multiple lines and not changing the expression nesting.

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented May 23, 2015

'bikeshed' is my middle name! Right now, it rejects more than 150 characters on a line. Even though people here are talking about 80 or 100 characters, for the sake of sanity, I am going to reduce that to 120 and see how it goes.

@frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented May 24, 2015

Just opened #6174 reducing line length from 150 to 120 characters

bors-servo pushed a commit that referenced this issue May 24, 2015
Part of #6041

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6174)
<!-- Reviewable:end -->
@nox
Copy link
Member

@nox nox commented May 25, 2015

The long lines that infuriate me the most are the patterns for EventTargetTypeId. Maybe we could also generate one value per leaf type? For example, generate a const TextTypeId equal to EventTargetTypeId::Node(NodeTypeId::CharacterData(CharacterDataTypeId::Text)).

@Ms2ger Ms2ger closed this Jul 2, 2015
jrmuizel pushed a commit to jrmuizel/gecko-cinnabar that referenced this issue Jun 12, 2017
…s (from frewsxcv:cleanup-long-lines); r=SimonSapin

Part of servo/servo#6041

Source-Repo: https://github.com/servo/servo
Source-Revision: 542519ebfd073662bc9421ac5fa0aa01ebc0d6fe
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Sep 30, 2019
…s (from frewsxcv:cleanup-long-lines); r=SimonSapin

Part of servo/servo#6041

Source-Repo: https://github.com/servo/servo
Source-Revision: 542519ebfd073662bc9421ac5fa0aa01ebc0d6fe

UltraBlame original commit: e2687fb0ec2e97a0e9e64c0591a07e3d3bca922c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 1, 2019
…s (from frewsxcv:cleanup-long-lines); r=SimonSapin

Part of servo/servo#6041

Source-Repo: https://github.com/servo/servo
Source-Revision: 542519ebfd073662bc9421ac5fa0aa01ebc0d6fe

UltraBlame original commit: e2687fb0ec2e97a0e9e64c0591a07e3d3bca922c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 1, 2019
…s (from frewsxcv:cleanup-long-lines); r=SimonSapin

Part of servo/servo#6041

Source-Repo: https://github.com/servo/servo
Source-Revision: 542519ebfd073662bc9421ac5fa0aa01ebc0d6fe

UltraBlame original commit: e2687fb0ec2e97a0e9e64c0591a07e3d3bca922c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.