dart_style icon indicating copy to clipboard operation
dart_style copied to clipboard

allow configuration of line length in pubspec.yaml

Open lukepighetti opened this issue 6 years ago • 101 comments

Hey @munificent is it possible to allow configuration of dartfmt line length in pubspec.yaml or some other common project configuration file? Of the last 5 flutter packages I've pulled down to work on 2 were using a 120 character line length which is becoming more and more common in Flutter projects.

I understand that we have access to IDE settings for line length but this is non-viable because it's IDE specific and not project-specific. Project specific would allow a package maintainer to make an increasingly common style choice and have all maintainers automatically use it without having to worry about IDE settings.

Any thoughts?

name: foo
description: An example app.
version: 1.0.0

environment:
  sdk: ">=2.6.0 <3.0.0"

dartfmt:
  line_length: 120

lukepighetti avatar Apr 15 '20 16:04 lukepighetti

Hi, Luke. I think you can probably guess what my thoughts are: https://github.com/dart-lang/dart_style/wiki/FAQ#why-cant-i-configure-it :)

If I were to add support for this, the very next day, all progress across the entire Dart ecosystem would screech to a halt as team members spent all day debating what line length should go into their package's config. I have seen no data to support the idea that programs vary such in their form such that it is measurably better to have different line lengths for different programs.

I took a corpus of the 994 most recent pub packages as of January. Of them, 763 contain ".dart" files with at least one line longer than 80 characters. When I exclude lines that start with "import", "export", or "part" that drops to 756. Skimming the result, most of long lines seem to be doc comments or long string literals, both of which aren't split by dartfmt.

If I exclude lines containing "//" or a quote character, then only 335 (34%) packages contain any files with a line over 80 characters.

I went ahead and looked at the individual lines of code in every file. Ignoring any trivial line that contains only whitespace or a punctuation character or two, I can bucket the remaining lines into "short" (<= 80 chars) and "long" (> 80).

212 (21%) packages have a ratio of 1% or less long lines to short. Only 62 (6%) packages have more than 5% of their non-trivial lines over 80 characters.

This says to me that most users are comfortable fitting within the current line length. And, even in packages that do go over 80 characters, it seems to rarely actually be helpful. Even packages that do have long lines, indicating that the user didn't format or formatted with a longer length have very few lines that actually exceed 80. That means the wider page size isn't buying them much except for wasted screen real estate on the majority of lines that don't benefit.

munificent avatar Apr 21 '20 00:04 munificent

I can see that you've spent a lot of time thinking about this. The problem with your stance is that projects already do make the decision whether to use the default line length or a custom one, the problem is that when they make that decision they aren't able to enforce it project wide. So not allowing line length settings in pubspec actually degrades the consistency of dartfmt in practice compared to leaving it up to IDE config files.

You even state that line length can be handled by the user, yet we still cannot set this project level.

https://github.com/dart-lang/dart_style/issues/833#issuecomment-518444299

lukepighetti avatar Apr 21 '20 17:04 lukepighetti

projects already do make the decision whether to use the default line length or a custom one

Some do, but most don't.

the problem is that when they make that decision they aren't able to enforce it project wide.

Yes, that is a valid problem for those teams. But part of maintaining an open source project like dartfmt is deciding which use cases to support and which to not. Time is finite and time spent adding this feature is time not spent doing other work that benefits other users.

In this particular case, my belief is that the feature has net negative value across the entire ecosystem. I have yet to see any evidence that a line length longer than 80 columns actually improves productivity or readability to any measurable degree. It is easy to demonstrate that going wider than 80 columns makes it harder to do side-by-side diffs or two-panel editing on laptop screens (which is many of us right now and even in the best of times, many users cannot afford large displays). We can directly observe the the time spent arguing over style choices in code reviews and elsewhere when users are given the ability to make style choices.

I understand that this particular issue is very close to your heart. But my very clear experience over several years of maintaining dartfmt, personally interacting with hundreds of users, and seeing dartfmt enforced across millions and millions of lines of code is that the best experience for all users is to just let it do its job, move on with your life, and focus on what your code does and not how it looks.

munificent avatar Apr 23 '20 18:04 munificent

@munificent Making dartfmt use line length from pubspec does not in any way create any issues for those developers with 80 char width devices. It is an easier way of enforcing 80 character limit for projects that want to enforce it, instead of asking every developer to change their ide settings or add the command line argument while formatting.

You can keep your side by side diffs for projects you work on. But don't enforce your constraints on other people. @lukepighetti isn't asking you to change the defaults. He is only asking for a very simple, straight and sensible way to do something which is already possible today but requires lot of manual efforts for everyone involved.

ajay-mm avatar Oct 06 '20 17:10 ajay-mm

As of today line length is user configurable but its fractured and unreliable because it can only be set in IDE configurations (like .vscode/settings.json) but it cannot be set in pubspec.yaml. If we could set it project-wide in pubspec.yaml then any user of any IDE would be able to minimize diffs when working on a project.

Also, the 80 character line limit is an arbitrary and unfair constraint on Flutter developers. A growing number of Flutter projects are switching to 120 character line limits in order to keep using dartfmt and get more pleasing results with deeper trees and longer class declarations with common mixins.

I have no problem if pub.dev wants to hammer scores if packages aren't 80 character line width, but for private projects it seems reasonable to fully implement this feature. If the 80 character width is so important, get rid of the ability to run dartfmt with a different line length.

lukepighetti avatar Oct 07 '20 11:10 lukepighetti

+1

israellias avatar Nov 15 '20 07:11 israellias

AVOID lines longer than 80 characters

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.

But programming is NOT a newspaper. If 80 chars line lenght is that great, why people keep asking to change that from pubspect (and in the meantime changing that per IDE instance)? Studies done on (newspaper) readers are applied to programers. I personally sometimes feel that 160 is too small, but thats what I am using for most of my projects.

+1, allow to change that and lower the "readibility" score.

Antoniossss avatar Dec 11 '20 22:12 Antoniossss

Please allow line length setting to be set in pubspec as not having this causes problems for anyone no wanting to use default 80 chars.

With ultrawides being more common and descriptive class/variable names, 80 chars limit is too low. We use 160 for all our projects.

crimsonvspurple avatar Aug 02 '21 11:08 crimsonvspurple

This is becoming very frustrating to deal with. We no-longer use 4:3 800x600 displays, and have 16:9, 16:10, or even 21:9 displays at high res which can comfortably use higher that 80 char lines, in multiple views.

80 char line limits become even more of a problem when it unnecessarily breaks a line that is fine at say 100 char because the variable names are perhaps a little longer for descriptiveness (e.r, status.isPermanentlyDenied). Such instances reduce readability quite drastically.

80 char line limit enforcement across a whole ecosystem is frankly baffling. And archaic.

flukejones avatar Aug 08 '21 23:08 flukejones

It's as simple as this:

80 char limit:

        builder:
            (BuildContext context, Box<MdlMachine> machines, Widget? child) {
          return OrientationBuilder(
              builder: (BuildContext context, Orientation ori) {
            var nameInBody = false;

100 char limit:

        builder: (BuildContext context, Box<MdlMachine> machines, Widget? child) {
          return OrientationBuilder(builder: (BuildContext context, Orientation ori) {
            var nameInBody = false;

I gain nothing from being forced in to 80 char limits here except for wasted lines. And the further you get in to flutter the worse it can get, particularly since widget properties can become verbose and are often required to be descriptive to convey their meanings and requirements easily.

I see vscode plugin at least has "dart.lineLength": 100,. But this really must be in the pubspec.yaml

flukejones avatar Aug 09 '21 00:08 flukejones

Please allow line length setting to be set in pubspec as not having this causes problems for anyone no wanting to use default 80 chars.

dartfmt does not have any awareness of pubspecs. Right now, it just formats each file individually completely context-free. This makes it easy and fast to run it in a variety of configurations. For example, many text editors pipe source to it through stdin. Changing dartfmt to scan parent directories for pubspecs would be a significant change in how it behaves.

80 char limit:

        builder:
            (BuildContext context, Box<MdlMachine> machines, Widget? child) {
          return OrientationBuilder(
              builder: (BuildContext context, Orientation ori) {
            var nameInBody = false;

100 char limit:

        builder: (BuildContext context, Box<MdlMachine> machines, Widget? child) {
          return OrientationBuilder(builder: (BuildContext context, Orientation ori) {
            var nameInBody = false;

Following the recommended style would give you:

        builder: (context, machines, child) {
          return OrientationBuilder(builder: (context, ori) {
            var nameInBody = false;

Ultimately, it's your code to style and format as you please. dartfmt is happy to format longer line lengths if you tell it to, and you certainly don't have to follow "Effective Dart". Our tools default to the style they do because we believe it's the most productive for the most users.

Is that default going to please all users? No. No default would. I've had users say they'll never use Dart again if we get rid of semicolons and others say that will never use Dart until it gets rid of them. You can't please everyone. But having nice defaults that most people agree on makes it easier for all of us to read each others' code, even if it's not in our personal preferred style.

munificent avatar Aug 10 '21 22:08 munificent

Default can be 80 or 60 or 100. That's is fine. I don't care about that.

I believe the reason people are posting here about it, is: You set longer line lengths. Commit push codes. Some day some other developer pulls, edits code, runs a format and things go back to default line length.

Basically, I (and probably many others) are asking for a way to have the line length property within the project itself so that regardless of who opens it wherever, code formatting stays the same. The default value or changing the default value in dart style guide is not related to this.

I have personally worked around it by committing select IDE project files in git for now.

crimsonvspurple avatar Oct 02 '21 14:10 crimsonvspurple

Just had a situation where someone submitted a PR to my repo with their own line length. If line length were project specific (defined in pubspec.yaml) and not left to IDE settings, I believe this could have been avoided.

I am faced with a situation where I feel compelled to create IDE specific configs in all my dart projects to try to enforce the 80 character default because people appear to be resorting to global IDE settings since there isn't a convention built around handling this at the project level in pubspec.yaml

https://github.com/lukepighetti/salomon_bottom_bar/pull/15

lukepighetti avatar Nov 22 '21 16:11 lukepighetti

Also, had a chat with a Kotlin dev the other day who was lamenting that we couldn't have Dart on the same line length as our Kotlin and Swift codebases because there was no way to configure it at the project level (in this case, 120 characters wide)

lukepighetti avatar Nov 22 '21 16:11 lukepighetti

Ran into this again today, this repo is using >80 char line length. I have no problems reading and manipulating the code. But because there is no pubspec.yaml setting for line length, my IDE is breaking formatting and increasing diffs unnecessarily. https://github.com/osaxma/dfs

lukepighetti avatar Jan 10 '22 16:01 lukepighetti

Copying my comment from here:

Dart format does not have any notion of "project" or "package" and doesn't know where to look for any sort of global or package-level configuration data. It just formats each file path you give in a completely context-free manner.

This is important because:

  • It's faster. The formatter doesn't look through parent directories for config files and determine which package a given file belongs to before it can determine how to format it.
  • It ensures you get consistent formatting when formatting files on disk or in your editor. A lot of editors invoke dart format by passing a string of code to it on stdin and reading the result back from stdout. For those invocations, there is no file path that the formatter could use to find the config file. We could require IDEs to pass in the desired line length, but that's likely to be brittle.

We have millions and millons of lines of Dart code inside Google formatted to 80 columns. I can absolutely promise you that Googlers have come up with class names and method names as long as or longer than anything you see in the wild. This is nowhere near the top of the priority list for improving Googlers' lives and I find it very hard to believe that configurable line length will measurably improve the lives of any external users either.

I have never seen code that I felt would be easier to read with a longer line length. I've seen a lot of code that would be easier to read if the author rethought the verbosity of some of their identifiers.

munificent avatar Jan 12 '22 02:01 munificent

The problem with your stance is that projects already do make the decision whether to use the default line length or a custom one, the problem is that when they make that decision they aren't able to enforce it project wide.

Exactly, I haven't been on a Flutter team that hasn't already had this debate. It usually lasts about 20 minutes, and we settle on 100 or 120.

This debate is already being had, we just don't have a way to enforce it. Which is much more of a productivity blocker than the initial debate, as each time a new team member is onboarded to a project, they start screwing up the git diffs.

esDotDev avatar Jan 12 '22 02:01 esDotDev

This debate is already being had,

This is true for some teams, but not all. If we add support for configuring it the formatter, than we will expand the set of teams that end up having this debate. That's an anti-goal for the formatter. One of the key reasons this software exists at all is to reduce the set of things teams debate.

Imagine that someone passed a law—literally a criminal offense—if you formatted your code longer than 80 columns. You were forced to comply. Do you honestly believe that your engineering productivity or overall quality of life as a human on Earth would be measurably negatively impacted? If not... why not just stick with 80 and spend your limited time on this spinning planet on stuff that matters?

munificent avatar Jan 12 '22 18:01 munificent

My stance is that configurable line length is half implemented. We should either remove the ability to format at custom line lengths entirely, or complete the implementation and allow it to be set at the project level

lukepighetti avatar Jan 12 '22 18:01 lukepighetti

Do you honestly believe that your engineering productivity or overall quality of life as a human on Earth would be measurably negatively impacted? If not... why not just stick with 80 and spend your limited time on this spinning planet on stuff that matters?

Measurably is the key word here, but yes, my day to day enjoyment on my ultrawide monitor is negatively impacted substantially when reading and writing Flutter code that needs to break at 80 chars.

Not so much package code, because package code tends to be less indented overall. But Flutter day-to-day UI code? It is quite annoying yes.

The reason all of my package code is still submitted at 120lines, is because I'm not about to start changing my IDE settings everytime I jump back and forth between projects and packages, which I would inevitably forget, and the results of that are not fun.

Could I live with it? Of course, but I don't find that a very convincing argument. This is a discussion about quality of life discussion where small things matter.

we will expand the set of teams that end up having this debate.

True, but you miss a key benefit in your overview here. For the teams that do have this debate, you will enhance their productivity because they can properly implement the results of their debate and be done with it.

esDotDev avatar Jan 12 '22 19:01 esDotDev

We should either remove the ability to format at custom line lengths entirely, or complete the implementation and allow it to be set at the project level

Many many many command line tools accept options on the command line that aren't also available from an implicitly discovered configuration file, so I don't see accepting the line-length argument as being an incomplete implementation.

For the teams that do have this debate, you will enhance their productivity because they can properly implement the results of their debate and be done with it.

At the expense of lowering the productivity of every user that switches between multiple codebases that would configure their line lengths differently. Every time you clicked "go to definition" on some imported library and ended up in a file whose line length doesn't fit in your IDE window, you'd have a bad experience.

The charter of dart_style is to help the Dart ecosystem, not just to improve the lives of individual developers. It is a public park, not a lawnmower you can use for your lawn. All are welcome to visit its park and because it is a shared park used by many, it can be particularly well manicured. But visiting it means accepting that maybe the grass is not cut to the exact length you might personally prefer. Hopefully, being able to explore an entire park (the entire package ecosystem) more easily makes that trade-off worth it.

munificent avatar Jan 12 '22 21:01 munificent

Many many many command line tools accept options on the command line that aren't also available from an implicitly discovered configuration file, so I don't see accepting the line-length argument as being an incomplete implementation.

Looking at the context of code formatters, dartfmt is the only one I'm aware of that has a line-length argument but no way to set it for the project outside of IDE config files. https://prettier.io/docs/en/options.html#print-width

lukepighetti avatar Jan 12 '22 22:01 lukepighetti

An advanced search of github for "max_width =" language:TOML reveals line widths from 80 up to 160, with the most common being 100, for the rustfmt.toml that rust uses. And somehow this absolutely does not affect the Rust ecosystem.

None of the arguments made to keep 80 column limit as a hard limit make sense and it just inconveniences the owners of the repos. The "But googlers don't need it" argument is not as good as you think - are you going to tell us to write shorter function and variable names to compensate for the forced line length also?

It is odd that a language as configurable as Dart, with a ridiculous variety of styles to write in, such that packages like "pedantic" are a thing, is getting so hung up on a single thing - line length. Change any one of these options but don't you dare go over 80 char wide for lines.

flukejones avatar Jan 12 '22 22:01 flukejones

At the expense of lowering the productivity of every user that switches between multiple codebases that would configure their line lengths differently. Every time you clicked "go to definition" on some imported library and ended up in a file whose line length doesn't fit in your IDE window, you'd have a bad experience.

No, not every user. Just users on very small screens, like 16" laptops. I don't know why we would optimize for the hobbyist developer that does not have a 2nd monitor attached. And if we are, why stop there, if you set it to 40 you could even include phone users. The obvious answer is that by going too short, you negatively impact users on large monitors, which is what we are complaining about here. It seems pretty arbitrary, especially in a world where programmers bill out at $5000/wk, but a larger monitor to make you more efficient, is $200.

visiting it means accepting that maybe the grass is not cut to the exact length you might personally prefer.

I take your point about consistency, but the analogy feels more like a park that is ostensibly build for adults, but all the rides are made for people 4ft tall :p

Maybe I'm being to cynical, but it really does feel like the underlying cause here is most of the googlers use laptops as their primary coding environment, so it's optimized for small screens, and all of this other stuff is just rationalizing reasons for that optimization.

But there's no point in debating the default, as it is so highly contextual. We should be able to set it on a per project basis though, if that is our wish, and in a way that does incur technical debt if we really do care about overall developer productivity.

esDotDev avatar Jan 12 '22 22:01 esDotDev

The "But googlers don't need it" argument is not as good as you think - are you going to tell us to write shorter function and variable names to compensate for the forced line length also?

What I am suggesting is that your code would be more readable to all who read it if you considered shorter function and variable names and hoisting more code out of deeply nested subexpressions into local variables or helper functions. Using a wider line length is treating the symptom.

I have seen a lot of code that is clearly improved by the author rethinking names and nesting structure when they find it line wraps too often. I've never seen code whose readability I felt was improved by making the lines wider. Just because you have a bigger screen, that doesn't mean you have bigger eyes or a bigger brain. Long identifiers and deep expressions still take more effort to read, regardless of how they line wrap.

If you prefer particularly long identifiers and deep expressions, that's your choice. It's your code. But I don't think it benefits the larger Dart ecosystem to tailor the tooling around that choice. And I've never been convinced that it's a good use of the time that Google pays me for to implement support for configuring that in dart_style when I could be spending that time on other things.

munificent avatar Jan 13 '22 17:01 munificent

@munificent that is of course very good advice and I don't doubt for a second that all of us here follow it.

You keep framing this as "googlers" and "ecosystem" though.

Googlers seem to have a different hardware setup that maybe justifies this choice - I don't, and neither do my work colleagues (we use ultrawides). We have a 60,000 line codebase which will never see the light of day outside our offices so "ecosystem" is not a concern for us.

Depending on the options you select for dart-analysis which is ridiculously configurable, this affects heavily how you write your code and has more impact on it than what may have been a previously okay-length var/func name.

For example setting unnecessary_new and unnecessary_this has an impact on line length. Same with always_declare_return_types and especially always_specify_types. Yet none of these are enforced, and depending on the rule-set package you use it affects how long your lines are. If you say "

Allowing the IDE like to pass a line-length to dartfmt, but not saving that length in a config for the project is confusing and risks a project ending up with odd formatting.

The enforced line length is completely at odds with the amount of configuration dart-analysis allows - and as rust has shown, it doesn't impact the ecosystem in the slightest.

flukejones avatar Jan 13 '22 21:01 flukejones

A "fairly" known Dart repo that uses lines much longer than 80 chars and don't even use dartfmt is of course the Flutter repo itself.

Which is kind of ironic since dart and flutter packages on pub get a score penalty if dartfmt and max 80 char line length is not used.

I think both @lukepighetti and @esDotDev made excellent points on why being able to set the line length on a project level for dartfmt would be very useful.

I would definitely prefer to use eg 120 on a project level too, because we use the lint that the Flutter repo does, the one that requires you to specify all types. This extra, but in our opinion nice formal verbosity, definitely adds to line lengths and causes more not so terrific looking line breaks. It would be nicer at 120, but we have stuck to 80 due to the fact that it cannot be set on project level.

rydmike avatar Jan 14 '22 12:01 rydmike

The truth is, leaving teams in a state of technical debt because they can not properly specifiy a team-wide setting like line-length, is an order of magnitude more disruptive than having a debate about line-length in the first place.

And as flukejones mentioned, line length is one of a dozen different discussions you need to have when discussing lint rules in dart for any project. (And usually one of the shorter, easier discussions, tbh) It's awesome that we can define all of these options as our team desires, but then bizarre that we are blocked on setting something as intrinsic to our workflows and production as preferred max line length. Not being able to set it leads directly to time-consuming issues with version control.

If the argument is truly about developer efficiency, there is no debate imo, this policy does far more harm than good.

I think the fact that the Flutter team opted not to use 80 char breaks flies pretty squarely in the face of "most people have no problem with it", which is not really a good argument anyways, when dart is so otherwise customizable, but even if it were a good argument, it doesn't seem to be true.

When combining: "always specify types" with Flutters "everything is a widget" philosophy, which means a lot of indentation, is it really surprising that 80chars is not working for us, when it works fine for pure dart devs?

esDotDev avatar Jan 14 '22 14:01 esDotDev

If you say " isn't recommended" then why is it an option to start with?

Many of the lint rules exist to help users migrate from previous versions of Dart to the latest features. Dart allows you to write new even though we recommend that no one does so that existing Dart code wasn't broken by using removing the new keyword.

munificent avatar Jan 18 '22 23:01 munificent

At this point it's clear whoever's in charge of this, isn't gonna budge. It's very likely the people from flutter had this debate with dart guys and since flutter doesn't use dartfmt; we can infer the outcome 😒

I'm glad I use Intellij where I can just commit settings file and have it format with 200 line length every time. I'm also glad I'm in position to enforce everyone in the team uses the same IDE.

If I ever push something in the "public park", people who don't like the size of the grass can stay out for all I care.

crimsonvspurple avatar Jan 19 '22 04:01 crimsonvspurple