dart-lang / dart_style

An opinionated formatter/linter for Dart code
https://pub.dev/packages/dart_style
BSD 3-Clause "New" or "Revised" License
640 stars 117 forks source link

Rethink the 80 characters line limit #833

Open feinstein opened 4 years ago

feinstein commented 4 years ago

I am not sure if this is the right repository to discuss this, but I would like to propose a investigation if 80 characters per line should still be the recommendation and dartfmt default.

The Dart style guide says:

Readability studies show that long lines of text are harder to read because your eye has to travel farther when moving to the beginning of the next line. This is why newspapers and magazines use multiple columns of text.

I think this is perfectly valid for regular text for newspapers and magazines, that usually come in the form of:

Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book.

But recently with the soaring success of Flutter, we are finding deeply nested code, which leads to lots of space being wasted as pure indentation. This picture shows how one of my Flutter widgets has just about half its line wasted in indentation, forcing me to break lines all the time:

image

Bear in mind, this is just a 9 Widget deep tree: Provider>Scaffold>SafeArea>WillPopScope>Column>Consumer>Padding>Row>Text.

And I didn't have any classes there with long names. If I did, it would have been a multi-lined mess.

IMHO, we can't use studies made for newspapers for coding styles, specially after the usage has become so different in the Flutter age.

I have seen people in the past defending 80 character lines saying it fits better all monitors, but nowadays with everyone using 24" monitors 1080+ I don't think that's longer the case.

I don't want to propose a new number, like 120 characters, but instead I would like to suggest that Google should launch a research on this, taking in consideration Flutter Apps and what line character count fits better their code.

My hope is that we don't turn out in the end with just 40 characters per line as real usable space due to indentations and enforced standards.

munificent commented 4 years ago

we are finding deeply nested code, which leads to lots of space being wasted as pure indentation.

Increasing the column limit doesn't really fix that problem. You're still wasting just as much space on the left side of the page. You're just able to make the window wider and eat space on the right side of the page that you could be using for other things.

Some amount of nesting is, of course, natural in Flutter. But I don't think any page width is going to make >10 levels of indentation a good idea. That's just too much nesting for the eye to follow. Instead, I think a better solution is:

I have seen people in the past defending 80 character lines saying it fits better all monitors, but nowadays with everyone using 24" monitors 1080+ I don't think that's longer the case.

I do all of my work on a 15" laptop and very much appreciate being able to do side-by-side diffs, so going wider than 80 columns would be a significant challenge for me.

feinstein commented 4 years ago

The problem is when we are obligated to run dartfmt before each commit. this way we can't escape the laws of indentation, unless the Flutter team releases a new flutterfmt tool to help with this.

Also, breaking down into smaller Widgets is just swapping one problem for another. We reset the indentation, but now we have lots of boilerplate to code. Helper methods are in general not recommended . Also, sometimes splitting the Widget tree makes it easier to read when you care about the high level, but it also can make it harder, if you are trying to follow down the rabbit hole on where's the problem, and you find yourself going back and fort in the file, looking for different Widget classes, so it can be helpful or not, depends on the context.

I am not saying I am against breaking the build method, I do it all the time, it's just that sometimes it just doesn't make much sense, and if we use more specialized Widgets, like Provider and Consumer the tree will get even more nested and harder to find a logical place to isolate the code.

munificent commented 4 years ago

The problem is when we are obligated to run dartfmt before each commit.

You can pass --line-length=<value> to dartfmt to specify a custom column limit for whatever value you want. 80 is default, but this is one of the few places where it allows you to override that.

feinstein commented 4 years ago

I generally use Ctrl+Alt+L in IntelliJ IDEA, can't see an option in the plugin to pass this configuration :/

deakjahn commented 4 years ago

@feinstein I think that piece of advice about helper functions is about something entirely different. Not using mere functions instead of actual widgets. This has its clear reasons but that doesn't in any way forbid you from using helper methods inside your widgets. Basically:

@override
Widget build(BuildContext context) => InkWell(
      child: Stack(
        children: [
          _displayIcon(),
          _displayName(),
        ],
      ),
      onTap: () {
        if (onTap != null) onTap();
      },
    );

Widget _displayIcon() => CachedNetworkImage(
  // ...
    );

Widget _displayName() => Align(
  // ...
    );

instead of lifting the CachedNetworkImage and the Align (and everything that comes below it) up to the first function. I do it all the time for readability and it makes my life much easier.

Having said that, I wholeheartedly agree with the lifting of the 80 character limit which I have always found hilariously low in the age of 28-30-whatever monitors. And even on laptops.

deakjahn commented 4 years ago

@feinstein by the way, if you go into Settings > Editor > Code style > Dart > Dartfmt, you'll see that it does pick up the width from another tab. Although I'm not sure it really does make any difference. :-)

feinstein commented 4 years ago

I know that changing my code helps, but I feel this is like Apple saying "you are holding it wrong".

I just don't want to write additional code when I think my actual code is OK, just too much to the right. It's like creating a new problem for fixing an old one.

greglittlefield-wf commented 4 years ago

As another data point, I did a search and found that ~35% of our organization's Dart projects (maintained by various independent teams) use a custom dartfmt line length of either 100 or 120 as opposed to the default 80.

And, almost all of those repos with custom line lengths contain over_react UI code, which has similar nesting/syntax to Flutter widgets.

keomaborges commented 4 years ago

PSR-12 defines the concepts of soft limit and hard limit:

There MUST NOT be a hard limit on line length.

The soft limit on line length MUST be 120 characters.

Lines SHOULD NOT be longer than 80 characters; lines longer than that SHOULD be split into multiple subsequent lines of no more than 80 characters each.

There MUST NOT be trailing whitespace at the end of lines.

Blank lines MAY be added to improve readability and to indicate related blocks of code except where explicitly forbidden.

There MUST NOT be more than one statement per line.

I think it is quite fair and looks nice for PHP. There might be something similar for Dart as well. In my opinion, 80 and 120 would be great.

munificent commented 4 years ago

As another data point, I did a search and found that ~35% of our organization's Dart projects (maintained by various independent teams) use a custom dartfmt line length of either 100 or 120 as opposed to the default 80.

Sure, but another way to look at that data point is that ~65% of your teams are using the 80 character line length, which implies that it's the right choice to standardize on.

ErliSoares commented 4 years ago

As another data point, I did a search and found that ~35% of our organization's Dart projects (maintained by various independent teams) use a custom dartfmt line length of either 100 or 120 as opposed to the default 80.

Sure, but another way to look at that data point is that ~65% of your teams are using the 80 character line length, which implies that it's the right choice to standardize on.

They are using 80 characters per line does not mean that they think it is better, most programmers I know do not agree that 80 is better, but use it as a guideline.

Part of the projects is 80 characters per line and part 120, but if the guideline changes to 120 everyone would use the same pattern.

deakjahn commented 4 years ago

No, Bob, it isn't. You can be as opinionated as you please in other corners of dartfmt, especially if, as right now, good placement of empty comments allows us to influence the formatting in many cases, but don't force line lengths on us, keep it a setting. Not everybody works in teams. Not everybody uses 14" laptop screens. Not everybody actually cares about old-fashioned ideas why the 80-limit should be better than anything larger of having no limit at all or whatever. It's really understandable that, for the sake of uniformity, you never wanted to introduce hundreds of user changeable settings. During the last year or so of working in Flutter I've grown to actually like the way dartfmt works (and in the few cases where I have differing opinion, I simple use those extra comments) but I'm sure to switch it off the minute you decide to rob the line length setting from us. I like working on a 30+" monitor and I want to use it all, not just the left quarter strip.

vincevargadev commented 4 years ago

There are advantages to embracing a universal line length limit in the community. It's the tabs-vs-spaces and other endless discussions happening all the time in all programming communities (e.g. Python's PEP8). This reminds me of the Go Proverb:

gofmt's style is no one's favorite, yet gofmt is everyone's favorite.

I don't really care if it's 80 or 100 characters, what I do not want is every package on pub.dev using different styling conventions and buggy, complex tools.

feinstein commented 4 years ago

As I said in my original post, the 80 character line was made for newspapers and magazines, so eye movement be constrained to a region, this way reading would be more comfortable and easy to matain attention on the text.

But in Flutter we get lots of whitespace indentations on the left, making the text more like a zigzag pattern, not the same as a newspaper.

So here's a crazy idea to explore and research:

80 character lines counted after the first non-whitespace character in that line (left most whitespaces should be ignored on the character count).

This could be coupled with IDE tools that auto-scrolls the text left and right, keeping it bit centered as we scroll it up and down (but not perfectly centered, so we still get a sense of indentation as we scroll).

Is this good? Will it work? Will code be easier to read? Is this ridiculous and will it fail miserably? I have no idea, but I would love to see some research done on it, that's how science evolves afterall, crazy hypothesis being tested so we can come up with new ideas and new hypothesis based of the previous ones.

deakjahn commented 4 years ago

@feinstein The 80-char has nothing to do with newspapers and magazines. That's an afterthought somebody tried to make up to justify that very old rule that simply goes back to the monitors of several decades ago.

feinstein commented 4 years ago

Independently, my suggestion is based on the fact that Flutter code has lots of leading whitespaces, whereas normal texts does not, so my suggestion stands.

feinstein commented 4 years ago

It appears that Linus Torvalds agrees with us:

https://www.theregister.com/2020/06/01/linux_5_7/

munificent commented 4 years ago

don't force line lengths on us, keep it a setting.

There are no plans to remove the option for specifying line length when running the formatter.

shinriyo commented 3 years ago

I want auto formatter when saving.

munificent commented 3 years ago

It appears that Linus Torvalds agrees with us:

https://www.theregister.com/2020/06/01/linux_5_7/

Linux uses 8 spaces of indentation, though, which Dart does not.

lukepighetti commented 3 years ago

This, to me, is the most frustrating pain point of Dart. We can run dartfmt with any line length we want, we can set up a single IDE to format with any line length we want, but we cannot enforce this project & IDE wide with a pubspec.yaml setting. I have no problem with https://pub.dev removing 20 points if someone doesn't use an 80 character line length. This isn't about public projects, this is about internal projects.

The only compelling argument I've heard against finalizing this feature is "because I don't want to," which honestly, is a fairly compelling argument in open source as far as I'm concerned, but it doesn't make it any less frustrating and senseless. I understand that maintaining dartfmt probably feels like taking on an army of slobbering screaming orcs with nothing but an xacto knife, but give the community a little bit of credit on this one @munificent .

MrCsabaToth commented 3 years ago

Even with 100 I can easily split the editor window vertically. 80 is so limited that it breaks even short+dumb code lines into multiple lines -> the brain has to work more cognitively to unwrap the riggidy broken lines.

azimuthdeveloper commented 3 years ago

Just dug this up. I have a line length configured on my computer, and another one configured on my laptop. I have to remember, in my brain, the line length I have set, otherwise, I save files, and then it makes a git diff a total nightmare.

Also, this comment:

I do all of my work on a 15" laptop and very much appreciate being able to do side-by-side diffs, so going wider than 80 columns would be a significant challenge for me.

That's nice, and would relevant if you were the only person that used Flutter to develop apps. But you're not. So it's not relevant. And even if it was too wide or whatever, you could configure it by setting it in the pubspec.yaml as required.

MrCsabaToth commented 3 years ago

Just dug this up. I have a line length configured on my computer, and another one configured on my laptop. I have to remember, in my brain, the line length I have set, otherwise, I save files, and then it makes a git diff a total nightmare.

Also, this comment:

I do all of my work on a 15" laptop and very much appreciate being able to do side-by-side diffs, so going wider than 80 columns would be a significant challenge for me.

I work on a laptop, 1920x1080 so nothing fancy and I can side-by-side 100 or even longer files.

Levi-Lesches commented 3 years ago

I see there are two main arguments: 80 characters is standard, > 80 characters can look nicer. I think having the option for both should make everyone happy.

Since we can already override dart format, can there be another lint option too? Right now it's lines_longer_than_80_chars or nothing, but I agree that there should be some limit in a project. How about adding lines_longer_than_100_chars and lines_longer_than_120_chars as well?

MrCsabaToth commented 3 years ago

To "80 characters is standard" - even in 1980 a Wyse 50 character terminal (https://en.wikipedia.org/wiki/Dell_Wyse) had 132 characters... I don't say it to be 132, but maybe at least a 100? I'm splitting my IDE editor section vertically and 100 is pretty fine. Of course making the default to be overrideable is the bare minimum.

munificent commented 3 years ago

Of course making the default to be overrideable is the bare minimum.

It already is and has always been. Simply pass --line-length to your preferred length.

MrCsabaToth commented 3 years ago

It already is and has always been. Simply pass --line-length to your preferred length.

I'm using that, but some tools don't know about that. For example code generation. Maybe I was wrong and I indeed want to raise the default default from 80. Many generated code still use 80 and then they get reformatted...

pinkasey commented 3 years ago

maybe analysis_options.yaml is a good place for it? it's got linter options, why not formatting options as well.

frankly, I don't care which files it's in. I just want to be able to maintain readable flutter code and spend less time in code-reviews grunting over lines that were auto-formatter to the worse.

munificent commented 3 years ago

maybe analysis_options.yaml is a good place for it? it's got linter options, why not formatting options as well.

The linter is built on top of the Dart analyzer. The latter looks at your entire Dart program and the libraries it imports, type checks it, and otherwise does full static analysis on. (This is why lints can be based on the static types of pieces of your code.) Thus it makes it fairly natural for the linter to also look for and process an analysis_options.yaml file associated with the package your code is in.

Doing this kind of analysis is also much slower and means your program needs to be in a fully-formed, generally compile-time error free state.

The formatter doesn't work that way. It only parses code and it handles each file completely separately. It doesn't care if your file has type errors, or if your file is even inside a pub package. It just takes in text files (which by convention end in .dart) and does stuff with them. This makes the formatter much faster to run, allows you to format code even while it contains type errors, lets you reformat packages that you haven't run pub get on, etc.

The trade-off is that it makes it harder to support any kind of "package-wide" configuration. The formatter doesn't know what a package is. Given a file to format, it doesn't search containing directories for anything like a pubspec.yaml or analysis_options.yaml file. It just formats the files you give it.

pinkasey commented 3 years ago

@munificent Thanks, that makes a lot of sense.

I've since found out that one can configure line-length both in AndroidStudio and in VSCode. That solves it for me - I've just agreed the line length with the other 3 members of my team, and we each set it in our IDE. If there was a configuration file that the IDE reads, that you can set this in, and add it to git so that all team-members have the same configuration - that would be the best.

Maybe what I'm asking for is an issue for the AndroidStudio and VSCode plugins.

ookami-kb commented 3 years ago

@pinkasey you can add configuration files to git, it's supported by default in both IDEs.

For VS Code it would be [PROJECT_DIR]/.vscode/settings.json with the following content:

{
    "dart.lineLength": 120,
    "editor.rulers": [
        120
    ],
    "[dart]": {
        "editor.rulers": [
            120
        ],
        "editor.formatOnSave": true,
    }
}

For Android Studio / IntelliJ Idea it would be [PROJECT_DIR]/.idea/codeStyles/Project.xml with something like this:

<component name="ProjectCodeStyleConfiguration">
  <code_scheme name="Project" version="173">
    <codeStyleSettings language="Dart">
      <option name="RIGHT_MARGIN" value="120" />
    </codeStyleSettings>
  </code_scheme>
</component>
sourcecaster commented 3 years ago

Should be line length limited? Absolutely yes. So many times I've seen if statements which were unreadable due to enormous length. Should it be 80 characters? Like old lovely DOS text screen resolution? Absolutely no I'm sure. Counter to previous example I've also seen a lot of splitted if conditions or assignment statements which made program code pretty unreadable too. And here's why:

If we think about the code we see on the screen (program code, not just a plain text which is quite different): our brain tries to split it to logical blocks for easier understanding. And those blocks are perceived vertically only. Means more lines of code we add (within one logical block) - harder for our brain to perform logical separation. Yes, it becomes easier to read each particular statement (condition set, assignments, method calls etc.), but there is a drawback - it becomes harder to perceive logic blocks of our program code which is arguably more important.

Being able to easily see program logic is more important than being able to easily read particular expressions, this is especially true in collaborative projects when people who didn't write the code do not yet actually understand how it works. So of course it doesn't mean you can have 256-characters length statements and be happy with it. But there must be a reasonable balance between these two major factors:

As for a suggestion I would personally say: having like 100-characters limit is more reasonable than 80-characters. And for sure more reasonable than something like 150-characters.

STG284 commented 2 years ago

The dartfmt tool considers a comment also the part of code and doesn't allow the line to cross the limit of 80 which is quite annoying, I suggest it should only check if code is crossing the limit or not.

Example: image

Results into --> image

bennovw commented 2 years ago

For VS Code users, open settings Cmd + , search dart length to set your line length. 🥳

image

deakjahn commented 2 years ago

The real problem is that when you publish your code on Pub, the system still expects the stupid 80 chars and refuses to accept anything else. In your own project, you can set it in the IDE all right. But each and every one of us who creates packages and plugins for the community have to accept this ugly code formatting. And as Munificent is the benevolent dictator of dartfmt, and he adores his 80 characters, nothing will ever change. 😁

Don't get me wrong, I deeply appreciate his work. It's just that single conviction of his that I'd be very happy without...

Kravchenko32 commented 2 years ago

When evaluating 80 character limit, just ignore any leading whitespace caused by nested indentation. This would be a healthy compromise. I think forced 80 character wide code is utterly unreadable.

Levi-Lesches commented 2 years ago

AFAIK part of the reason for the 80-char convention is to ensure code does not stretch off the screen, and ignoring indentation won't really help with that.

EDIT: I agree that the limit should be increased to 100 or 120, I was just pointing out that ignoring indentation, especially tabs that can take 2 or 4 visual spaces each, wouldn't solve the problem.

MrCsabaToth commented 2 years ago

AFAIK part of the reason for the 80-char convention is to ensure code does not stretch off the screen, and ignoring indentation won't really help with that.

The question is what screen? A 40 year old WYSE 50 character terminal? My daily driver is a laptop, and I can split my IDE's edit screen vertically and still have two 100 or 120 character files next to each other. (Maybe if someone considers a 13" laptop as a daily driver cannot do that). I accept 80 if it can be overridden. Not so long ago I learned that packages on pub.dev have that 80 enforced, so maybe it's a Don Quixote fight to try to make the limit reconfigurable everywhere system wide.

There's some wiggle room to operate with extra commas to make the source code break nicely. I can also interpret ugly broken source that my cyclomatic complexity is too high.

gsouf commented 2 years ago

Apparently it's even nearly 100 years old standard. Source: https://softwareengineering.stackexchange.com/questions/148677/why-is-80-characters-the-standard-limit-for-code-width

Just saying...

deakjahn commented 2 years ago

Or even earlier, back to typewriters. Which all mean nothing today, really...

munificent commented 2 years ago

AFAIK part of the reason for the 80-char convention is to ensure code does not stretch off the screen, and ignoring indentation won't really help with that.

It's partially that, but also because the longer lines are, the harder it is for our eyes to correctly track from the end of one line to the beginning of the next. This is why newspapers print in several columns of text even though the pages are large.

deakjahn commented 2 years ago

@munificent Please, don't. I have 30 years of background in press and typography and this is but a myth. Obviously it applies to very wide text but nobody ever wanted to print a tabloid in a single column. Narrow columns had technical reasons, dating back to hand setting and Linotype and had absolutely nothing to do with readability. Books have much larger columns and still present no problems for us whatsoever. Kids in the first year of school might have problems but we learn to deal with reasonable column sizes very soon.

Don't use this as a justification. First, source code is not flowing text that should be read as such. Second, everybody is at perfect liberty in any IDE to increase line height if this is a problem for them.

gpeal commented 2 years ago

I don't think it's particularly useful to try and make an argument that 80 lines should always or never be used. Ultimately, I think the ask here would be to allow teams to make their own decisions via configuration.

feinstein commented 2 years ago

It's already possible to pass a flag for this configuration. The original post was about what character limit per line should be the official recommendation or default option.

For example, in my team we use 1000 characters as a limit in the dart format tool (PRs aren't allowed to be merged if there's a formatting issue), which is basically just saying that we don't want to limit.

Why we don't limit? Because we believe breaking code lines at each 80 characters can make code harder to read, which is the opposite of the original intention of the 80 characters limit. In my team the devs usually make cleaner line breaks than the tool with 80 characters, but I don't know if this applies to everyone.

MrCsabaToth commented 2 years ago

It's already possible to pass a flag for this configuration. The original post was about what character limit per line should be the official recommendation or default option.

There is a command line flag but you cannot bubble your custom settings down to everywhere in the toolchain. In some places the 80 still pops up.

Another thing is that I would like more detailed command line switch. My apps have generated code (Floor and Mockito generate code). These of course use the default 80, and once that goes through my custom setting ti unnecessarily changes those files.

gsouf commented 2 years ago

I think the main problem is rather pub.dev warning, isn't it?

deakjahn commented 2 years ago

If it was just a warning... But it's a hard error.

rafaelcolladojr commented 2 years ago

Let's just meet half-way and make the default 100. Can't tell you how many times my lines have been split because of a couple (<=10) characters...

Kravchenko32 commented 2 years ago

Let's just meet half-way and make the default 100. Can't tell you how many times my lines have been split because of a couple (<=10) characters...

Just stop using Dart. After a week of making changes to mangled flutter code we have, I’m done, it’s utterly unreadable. 80 character limit isn’t the issue though, the issue is that bot formats your code, and all bots are tone-deaf and have zero understanding of what’s human readable or which parts of code are actually important.