Help try out the new and improved Dart formatter!

[Bob Nystrom]

Greetings, Dartisans!

I'll just get the call to action out of the way first:

I've made a ton of improvements to the formatter, but I'd like you to try it out before I ship it in the next version of the SDK. To give it a shot:

1. Install the release candidate of the Dart formatter:
   
   $ pub global activate dart_style 0.2.0-rc.1
   
   
2. Run it on some of your code:
   
   $ pub global run dart_style:format <file paths...>
   
   
3. Let me know if you see anything busted in the output, crashes, stalls, etc.

The brave/foolhardy are also welcome to start using this version for real. If you decide to do that, make sure everyone on your team does too.

Assuming all goes well, I'll kick out a real 0.2.0 release and bring that into the SDK soon. I've run the new formatter on lots of code, but I'd still like to get more validation before I foist it onto the world.

Background

I've been deep in a dark code hole for the past several months hacking away on the formatter (dartfmt). Seriously, look at these commits. The formatter had some fundamental limitations that made it impossible to fix the bugs that were piling up. It took damn near a whole rewrite, but those limitations have been removed and the formatter now has a much more sophisticated splitting engine.

Once I got that working, I could finally fix some old bugs that had been driving me crazy. After that, I plowed through a bunch more bugs.

Despite all of that, most of your code will look the same. This is about improving the output in the 10% of the cases where it was doing a bad job. The other 90% is the same.

There are too many fixes to count, but here's some big ones:

Long argument and parameter lists

The old formatter would just keep packing in arguments as tightly as it could, leading to an unreadable blob of text. Now, if the list doesn't fit in two lines, each argument or parameter gets moved to its own line.

Old:

analysisServer = new AnalysisServer(serverChannel, resourceProvider,
    new OptimizingPubPackageMapProvider(resourceProvider, defaultSdk),
    index, serverPlugin, analysisServerOptions, defaultSdk,
    instrumentationService, rethrowExceptions: false);

New:

analysisServer = new AnalysisServer(
    serverChannel,
    resourceProvider,
    new OptimizingPubPackageMapProvider(resourceProvider, defaultSdk),
    index,
    serverPlugin,
    analysisServerOptions,
    defaultSdk,
    instrumentationService,

    rethrowExceptions: false);

Complex nested function calls

It does a better job of splitting outer argument lists to highlight the call structure:

Old:

Element element = new Element(ElementKind.ENUM_CONSTANT, name, Element

        .makeFlags(

            isPrivate: Identifier.isPrivateName(name),

            isDeprecated: _isDeprecated(node)),

    location: _getLocationNode(nameNode));

New:

Element element = new Element(

    ElementKind.ENUM_CONSTANT,

    name,

    Element.makeFlags(

        isPrivate: Identifier.isPrivateName(name),

        isDeprecated: _isDeprecated(node)),

    location: _getLocationNode(nameNode));

Big collection literals

Often, you're creating a bunch of data inside a large collection literal, something complex enough that you have line comments inside explaining its structure. Now, the formatter will preserve your newlines inside that to maintain the structure:

Old:

class Foo {

  /**

   * Basic one-byte 608 CC char set, mostly ASCII.

   * Indexed by (char-0x20).

   */

  static const List<int> ccUtfTable0 = const <int>[

    0x20,

    0x21,

    0x22,

    0x23,

    0x24,

    0x25,

    0x26,

    0x27, //   ! " # $ % & '

    0x28,

    0x29, // ( )

    0xE1, // 2A: 225 'á' "Latin small letter A with acute"

    0x2b,

    0x2c,

    0x2d,

    0x2e,

    0x2f, //       + , - . /

    0x30,

    0x31,

    0x32,

    0x33,

    0x34,

    0x35,

    0x36,

    0x37, // 0 1 2 3 4 5 6 7

    0x38,

    0x39,

    0x3a,

    0x3b,

    0x3c,

    0x3d,

    0x3e,

    0x3f, // 8 9 : ; < = > ?

    0x40,

    0x41,

    0x42,

    0x43,

    0x44,

    0x45,

    0x46,

    0x47, // @ A B C D E F G

    0x48,

    0x49,

    0x4a,

    0x4b,

    0x4c,

    0x4d,

    0x4e,

    0x4f, // H I J K L M N O

    0x50,

    0x51,

    0x52,

    0x53,

    0x54,

    0x55,

    0x56,

    0x57, // P Q R S T U V W

  ];

}

(Ugh, I know, right?)

New:

class Foo {

  /**

   * Basic one-byte 608 CC char set, mostly ASCII.

   * Indexed by (char-0x20).

   */

  static const List<int> ccUtfTable0 = const <int>[

    0x20, 0x21, 0x22, 0x23, 0x24, 0x25, 0x26, 0x27, //   ! " # $ % & '

    0x28, 0x29, // ( )

    0xE1, // 2A: 225 'á' "Latin small letter A with acute"

    0x2b, 0x2c, 0x2d, 0x2e, 0x2f, //       + , - . /

    0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, // 0 1 2 3 4 5 6 7

    0x38, 0x39, 0x3a, 0x3b, 0x3c, 0x3d, 0x3e, 0x3f, // 8 9 : ; < = > ?

    0x40, 0x41, 0x42, 0x43, 0x44, 0x45, 0x46, 0x47, // @ A B C D E F G

    0x48, 0x49, 0x4a, 0x4b, 0x4c, 0x4d, 0x4e, 0x4f, // H I J K L M N O

    0x50, 0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, // P Q R S T U V W

  ];

}

Nested functions and collections

The indentation rules for functions and collections are more sophisticated and allow them to indent like expressions in contexts where that produces a more readable result. It's hard to track down a good before/after, but in big nested expressions full of higher-order functions, you'll often see better results.

There's tons of other little fixes too, so let me know how it goes.

Cheers!

- bob

[Kasper Peulen]

Looks way better ! +1

--
For other discussions, see https://groups.google.com/a/dartlang.org/
 
For HOWTO questions, visit http://stackoverflow.com/tags/dart
 
To file a bug report or feature request, go to http://www.dartbug.com/new

To unsubscribe from this group and stop receiving emails from it, send an email to misc+uns...@dartlang.org.

--

Kasper

[Cristian Garcia]

Thanks!!! Does Webstorm get this for free or do they have to update something?

[Bob Nystrom]

On Fri, Jun 19, 2015 at 3:22 PM, Cristian Garcia <cgarc...@gmail.com> wrote:

Does Webstorm get this for free or do they have to update something?

When this lands in the SDK, I believe it will pick it up from there.

Cheers!

- bob

[Bob Nystrom]

On Fri, Jun 19, 2015 at 3:20 PM, Kasper Peulen <kasper...@gmail.com> wrote:

Looks way better ! +1

\o/

- bob

[DoHyung Kim]

Amazing job! I'll try it right now. Fortunately (?) I'm the only person working on the current codebase. ;)

DH

[Alexander Doroshko]

Right click in the editor -> Reformat with Dart Style. Apparently
behavior depends on the configured SDK version.

[Günter Zöchbauer]

I quite like the old one already, can't wait to try the new :) 

[Filipe Morgado]

You should read the original post again :P

[Thomas Schranz]

Wow, great job :)

[DoHyung Kim]

I took snapshot of the `analysis_server` located under `pkg/analysis_server` in the `github.com/dart-lang/sdk` after upgrading `dart_style` to `0.2.0-rc.1` in `pubspec.yaml`, and placed it under `<dart-sdk>/bin/snapshots`. WebStorm is now using the new `dart_style`. If anyone needs immediately application of the new version, it can be done fairly easily.

2015년 6월 20일 토요일 오전 7시 22분 25초 UTC+9, Cristian Garcia 님의 말:

[Bob Nystrom]

On Sat, Jun 20, 2015 at 6:55 AM, Thomas Schranz <tho...@blossom.io> wrote:

Wow, great job :)

\o/

- bob

[DoHyung Kim]

Bob,

In the previous version of dart_style, there was a blank line between class declaration and the following member with doc comments like the following:

class Test {

           <- This blank line

  /// The first member in this class.

  int id;

}

If the doc comment is missing, any blank line between class declaration and the first member was removed.
Now it seems that the blank line is removed in 0.2.0-rc.1. Is it intentional? I happen to have relied on the feature to have the single blank line.

Thanks.
2015년 6월 20일 토요일 오전 6시 21분 42초 UTC+9, Bob 님의 말:

[Andreas Kirsch]

Why do you want to have a single blank line there?

--
For other discussions, see https://groups.google.com/a/dartlang.org/
 
For HOWTO questions, visit http://stackoverflow.com/tags/dart
 
To file a bug report or feature request, go to http://www.dartbug.com/new

To unsubscribe from this group and stop receiving emails from it, send an email to misc+uns...@dartlang.org.

--

Why is this e-mail so short? Answer: five.sentenc.es.

[Bob Nystrom]

On Tue, Jun 23, 2015 at 7:26 AM, DoHyung Kim <dyn...@gmail.com> wrote:

Now it seems that the blank line is removed in 0.2.0-rc.1. Is it intentional? I happen to have relied on the feature to have the single blank line.

Yes, it's intentional. The relevant commit is:

https://github.com/dart-lang/dart_style/commit/1e802c7895c7236ac77aa3adfc96a9d8486d49f7

In general, the formatter's goal is to be as prescriptive as possible in order to make the resulting code as consistent as possible. This means that in most cases, it completely discards the original whitespace and produces its own. However, it can't do this everywhere. In particular, we humans often use blank lines to separate a series of statements or declarations into meaningful groups, like:

  void applySplit(Rule rule, int indent, NestingLevel nesting,

      {bool flushLeft, bool spaceWhenUnsplit, bool isDouble}) {

    if (flushLeft == null) flushLeft = false;

    if (spaceWhenUnsplit == null) spaceWhenUnsplit = false;

    if (isHardSplit || rule is HardSplitRule) {

      // A hard split always wins.

      _rule = rule;

    } else if (_rule == null) {

      // If the chunk hasn't been initialized yet, just inherit the rule.

      _rule = rule;

    }

    <blank line>

    // Last split settings win.

    _flushLeft = flushLeft;

    _nesting = nesting;

    _indent = indent;

    <blank line>

    _spaceWhenUnsplit = spaceWhenUnsplit;

    <blank line>

    // Pin down the double state, if given and we haven't already.

    if (_isDouble == null) _isDouble = isDouble;

  }

So the formatter preserves those. But in cases where we are sure a blank line either always adds value or never does, the formatter will now ignore the original whitespace. In particular, it removes any blank lines after a { and always adds a blank line above and below a class declaration.

In most code, this doesn't make much of a difference—most users already formatted their code this way anyway. However, in automatically generated code that may have no whitespace, it's really nice for the formatter to add some extra blank lines where appropriate.

Cheers!

- bob

[DoHyung Kim]

To me, the blank line helps me identify the first member more clearly. Especially when I use '//' instead of '/*'. You can see plenty of projects in languages like Java doing so. Anyway I'm ok with the current behavior of removing the blank line.

[DoHyung Kim]

I hope to add that I am even sold to Go's extremely strict standar formatting. So prefer stricter formatting rule. The reason I posted the original question is just to check if the observed deviation from the previous behavior was intentional or accidental.

[Filipe Morgado]

I don't use Dart formatter. (Haven't used Dart in a team yet)

Formatting is always very opinionated, 

I know, from other languages, that it's hard to sell strict formatting rules to whole teams.

Managers usually just select a single tool (or rules) and the team adhere to it, some may not.

Dart welcomes programmers from a lot of backgrounds. I'm glad it doesn't enforce any kind of formatting.

And it would take a configuration file bigger than the code itself to flexibilize a formatter to fit everybody's needs.

Details, when they don't affect readability, are irrelevant IMO.

But again, readability is itself very opinionated.

[Bob Nystrom]

On Tue, Jun 23, 2015 at 6:49 PM, DoHyung Kim <dyn...@gmail.com> wrote:

I hope to add that I am even sold to Go's extremely strict standar formatting. So prefer stricter formatting rule. The reason I posted the original question is just to check if the observed deviation from the previous behavior was intentional or accidental.

It's a totally valid question. Thank you for bringing it up so other people know about it too. :)

Cheers!

- bob

[Anders Holmgren]

Well that saves my filing an issue on dart_style. In general the old version did a pretty good job except for this

    testCase('when router has pathAdapters',

        forBuilder: () => (new DefaultRouterBuilder('top',

            routeAdapter: new DefaultRouterAdapter.def(

                pathAdapter: testPathAdapter('::')))

      ..get('foo', testHandler('one'))

      ..addAll((DefaultRouterBuilder r) => r

        ..addAll((DefaultRouterBuilder r) => r

          ..put('sub1sub1', testHandler('sub1sub1')),

            path: 'subr1subr1', pathAdapter: testPathAdapter(';;')),

          path: 'subr1')),

            expectRouter: createRouter('top::', [

      simple('foo::', ['GET'], true, 'one'),

      createRouter('subr1::', [

        createRouter(

            'subr1subr1;;', [simple('sub1sub1;;', ['PUT'], true, 'sub1sub1')])

      ])

    ]),

        expectBehaviour: [

      behaviour('a request matching first route',

          forRequest: () => createRequest('top::/foo::'),

          expectResult: testResult(result: 'one')),

      behaviour('a request matching second route',

          forRequest: () =>

              createRequest('top::/subr1::/subr1subr1;;/sub1sub1;;', 'PUT'),

          expectResult: testResult(result: 'sub1sub1'))

    ]);

where the indenting is completely wrong and misleading. The new one however, is spot on

testCase('when router has pathAdapters',
    forBuilder: () => (new DefaultRouterBuilder('top',
        routeAdapter: new DefaultRouterAdapter.def(
            pathAdapter: testPathAdapter('::')))
        ..get('foo', testHandler('one'))
        ..addAll(
            (DefaultRouterBuilder r) => r
              ..addAll((DefaultRouterBuilder r) =>
                      r..put('sub1sub1', testHandler('sub1sub1')),
                  path: 'subr1subr1', pathAdapter: testPathAdapter(';;')),
            path: 'subr1')),
    expectRouter: createRouter(
        'top::',
        [
      simple('foo::', ['GET'], true, 'one'),
      createRouter('subr1::', [
        createRouter('subr1subr1;;', [
          simple('sub1sub1;;', ['PUT'], true, 'sub1sub1')
        ])
      ])
    ]),
    expectBehaviour: [
      behaviour('a request matching first route',
          forRequest: () => createRequest('top::/foo::'),
          expectResult: testResult(result: 'one')),
      behaviour('a request matching second route',
          forRequest: () =>
              createRequest('top::/subr1::/subr1subr1;;/sub1sub1;;', 'PUT'),
          expectResult: testResult(result: 'sub1sub1'))
    ]);

great job and thanks for saving me having to create an issue ;-)

[Bob Nystrom]

\o/

I'm glad the formatter does a good job, but, man, code like yours is what keeps me up at night. That is one hell of a statement!

- bob

[Anders Holmgren]

Haha, yeah there were so many variants to testing the routing that I ended up creating a bit of a testing dsl for it. I guess you could make a case it ended up a little on the complex side ;-)

[Lasse Damgaard]

Just ran the formatter on a semi-large codebase and it generally looks great! Very nice work Bob!

I only really found one example where the new formatter doesn't work as well or better.

In 1.7:

    _trigger.then(ut.expectAsync((result) {

      if (_deferExpectations == null || _deferExpectations == false) {

        body(result);

      } else {

        defer(ut.expectAsync(() => body(result)));

      }

    })).catchError(ut.fail);

In 0.2.0-rc1:

    _trigger

        .then(ut.expectAsync((result) {

      if (_deferExpectations == null || _deferExpectations == false) {

        body(result);

      } else {

        defer(ut.expectAsync(() => body(result)));

      }

    }))

        .catchError(ut.fail);

On Friday, June 19, 2015 at 11:21:42 PM UTC+2, Bob wrote:

[Bob Nystrom]

Ah, yes. This is another case of: https://github.com/dart-lang/dart_style/issues/367. I added your example there. I'll see if the formatter can be smarter about this case, though it may be hard to do so without making other cases worse.

Cheers!

- bob