src: update cppline limit to 100

[jasnell]

The existing 80 has become rather restrictive.

[Trott]

@nodejs/cpp-reviewers

[RaisinTen]

LGTM.

node/.clang-format

Line 47 in da8f618

ColumnLimit: 80

also needs to be updated to 100 now.

[bnoordhuis]

It's deliberately restrictive, always has been. It forces you to write more succinct code.

Longer line lengths optimize for writability, shorter line lengths for readability. Code is read more often than it is written.

[tniessen]

My main concern right now is the format-cpp task. I've had to update PRs countless times because format-cpp wanted me to rewrite surrounding, existing code to match its code style. At least, once code has been committed, it won't have formatting issues. Now I am wondering if changing this rule will negatively impact progress there.

[jasnell]

At least, once code has been committed, it won't have formatting issues. Now I am wondering if changing this rule will negatively impact progress there.

I'm not convinced it would be an issue. At least, so far the experience with the quic pr has been pretty painless with this. I'd suggest we could go with this as an experiment. Rolling it back would be fairly straightforward if necessary.

[bnoordhuis]

I mulled this over but I really don't like flowery prose - you know the kind, interminable run-on sentences that take up half a page - and the same goes for code. Hemingway > *

[jasnell]

Added tsc-agenda for discussion. If there are technical reasons not to make the change, that's one thing, but the objections raised so far appear purely subjective. @tniessen raised a possible technical issue that bears consideration.

[MoLow]

just for the sake of discussion, I disagree with

Shorter line lengths optimize for readability.

in many occasions reading the beginning of a line can spare you from reading the entire line.
breaking a line of code into multiple lines force your eyes to read what could have been mentaly part of a shorter line.

for example, SomeClass::SomeMethod(with, a, list, of, arguements); is more readable than

SomeClass::SomeMethod(with, 
                      a, 
                      list, 
                      of, 
                      arguements);

especially when SomeClass::SomeMethod is named in a good and descriptive name

[jasnell]

Case in point, in the quic PR, I had to shorten...

  int err = nghttp3_conn_add_write_offset(connection_.get(), stream_data->id, datalen);

to

  int err = nghttp3_conn_add_write_offset(connection_.get(),
                                          stream_data->id,
                                          datalen);

I find the shorter lengths far less readable than the longer alternate.

[cjihrig]

I find the shorter lengths far less readable than the longer alternate.

Looking at it in isolation in the GitHub UI, the single line is more readable. As someone who exclusively works on a small screen and frequently looks at git diffs in my terminal, I disagree that it's more readable while I'm trying to do actual work though.

[Trott]

just for the sake of discussion, I disagree with

Shorter line lengths optimize for readability.

I dislike restricting the line length to 80 characters in general, but nonetheless the research that has been done bears out what Ben said: Keeping the lines to 80 chars or less improves readability.

Granted, almost everyone who cites the research cites (I think) a single study (but multiple papers) by Buse & Weimer. So you might argue that the data on this topic is thin. You might also argue that a study done in 2008 may not reflect the reality of modern code bases. You might also argue that maintainability is different (albeit related) to readability and this improves maintainability because (for example) we don't have to reformat lines copied/rebased/cherry-picked/whatever from other code bases. And so on.

But for Ben's point, what data there is backs up Ben on this as far as I am aware. Keeping most lines shorter than 80 characters correlates strongly with code readability (although there were some other elements that correlated more strongly in Buse & Weimer's research). And I say that as someone who dislikes 80 character line length limits. (100 or 120 seems reasonable to me.)

[GeoffreyBooth]

We went through this whole debate for JavaScript in #41509 and #41586. I think the C++ rule should just match the JS one, where we settled on 120 characters (for now).

I would also support upping the limit on Markdown either to match or for Markdown to have no limit.

[bnoordhuis]

the objections raised so far appear purely subjective

I mean, of course - the compiler doesn't care about line length but I, as someone who reviews a lot of code, definitely do.

W.r.t. #45033 (comment)

I find the shorter lengths far less readable than the longer alternate.

For me it's completely the other way around. My eyes just kind of start to glaze over 2/3rds in.

[tniessen]

@tniessen raised a possible technical issue that bears consideration.

@jasnell Just to clarify my (possibly unfounded) concern: C++ PRs have often been much larger than necessary recently to pacify format-cpp, which requires surrounding lines to be formatted properly. That means that a one line diff can turn into reformatting an entire function. (Because make format-cpp does not work nicely in some git workflows, I tend to postpone it and then have to update the PR just to pacify format-cpp.) I am wondering if this change would cause more such unnecessarily large diffs because clang-format might want to unwrap lines that fit into the new limit but not into the old.

[jasnell]

@tniessen ... ah, yeah, that's a good point. I had no intention of making it so that existing code would need to change but adjusting the formatter would absolutely have that impact... and for that reason alone I'll withdraw this.

[tniessen]

(On the other hand, if this is something that we'd want to do eventually, I don't know if waiting is going to give us any benefit.)