Skip to content

Commit c327b36

Browse files
committed
Clean up README and TODO comments.
Updated the example in the README to one that uses the new style. I also took the opportunity to find a more interesting example and generally rewrote things based on how the formatter is used today. When we were in the middle of the rewrite, I used "TODO(tall)" and "TODO(perf)" comments to track things I wanted to look into before I considered the rewrite "done". Now that the rewrite is basically good enough, I went through those. If I think the TODO is no longer worth doing at all, I just deleted it. Otherwise, I turned it into a general "TODO(rnystrom)" for us to keep in mind at some unspecified time.
1 parent cf0954a commit c327b36

File tree

9 files changed

+74
-85
lines changed

9 files changed

+74
-85
lines changed

README.md

Lines changed: 54 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
The dart_style package defines an automatic, opinionated formatter for Dart
2-
code. It replaces the whitespace in your program with what it deems to be the
3-
best formatting for it. Resulting code should follow the [Dart style guide][]
4-
but, moreso, should look nice to most human readers, most of the time.
1+
The dart_style package defines an opinionated automated formatter for Dart code.
2+
It replaces the whitespace in your program with what it deems to be the
3+
best formatting for it. It also makes minor changes around non-semantic
4+
punctuation like trailing commas and brackets in parameter lists.
5+
6+
The resulting code should follow the [Dart style guide][] and look nice to most
7+
human readers, most of the time.
58

69
[dart style guide]: https://dart.dev/guides/language/effective-dart/style
710

@@ -14,80 +17,89 @@ The formatter turns code like this:
1417

1518
```dart
1619
// BEFORE formatting
17-
if (tag=='style'||tag=='script'&&(type==null||type == TYPE_JS
18-
||type==TYPE_DART)||
19-
tag=='link'&&(rel=='stylesheet'||rel=='import')) {}
20+
process = await Process.start(path.join(p.pubCacheBinPath,Platform.isWindows
21+
?'${command.first}.bat':command.first,),[...command.sublist(1),'web:0',
22+
// Allow for binding to a random available port.
23+
],workingDirectory:workingDir,environment:{'PUB_CACHE':p.pubCachePath,'PATH':
24+
path.dirname(Platform.resolvedExecutable)+(Platform.isWindows?';':':')+
25+
Platform.environment['PATH']!,},);
2026
```
2127

2228
into:
2329

2430
```dart
2531
// AFTER formatting
26-
if (tag == 'style' ||
27-
tag == 'script' &&
28-
(type == null || type == TYPE_JS || type == TYPE_DART) ||
29-
tag == 'link' && (rel == 'stylesheet' || rel == 'import')) {}
32+
process = await Process.start(
33+
path.join(
34+
p.pubCacheBinPath,
35+
Platform.isWindows ? '${command.first}.bat' : command.first,
36+
),
37+
[
38+
...command.sublist(1), 'web:0',
39+
// Allow for binding to a random available port.
40+
],
41+
workingDirectory: workingDir,
42+
environment: {
43+
'PUB_CACHE': p.pubCachePath,
44+
'PATH':
45+
path.dirname(Platform.resolvedExecutable) +
46+
(Platform.isWindows ? ';' : ':') +
47+
Platform.environment['PATH']!,
48+
},
49+
);
3050
```
3151

3252
The formatter will never break your code—you can safely invoke it
3353
automatically from build and presubmit scripts.
3454

35-
## Using the formatter
55+
## Formatting files
3656

3757
The formatter is part of the unified [`dart`][] developer tool included in the
38-
Dart SDK, so most users get it directly from there. That has the latest version
39-
of the formatter that was available when the SDK was released.
58+
Dart SDK, so most users run it directly from there using `dart format`.
4059

4160
[`dart`]: https://dart.dev/tools/dart-tool
4261

4362
IDEs and editors that support Dart usually provide easy ways to run the
44-
formatter. For example, in WebStorm you can right-click a .dart file and then
45-
choose **Reformat with Dart Style**.
63+
formatter. For example, in Visual Studio Code, formatting Dart code will use
64+
the dart_style formatter by default. Most users have it set to reformat every
65+
time they save a file.
4666

4767
Here's a simple example of using the formatter on the command line:
4868

49-
$ dart format test.dart
69+
```sh
70+
$ dart format test.dart
71+
```
5072

51-
This command formats the `test.dart` file and writes the result to the
73+
This command formats the `test.dart` file and writes the result back to the same
5274
file.
5375

54-
`dart format` takes a list of paths, which can point to directories or files. If
55-
the path is a directory, it processes every `.dart` file in that directory or
56-
any of its subdirectories.
76+
The `dart format` command takes a list of paths, which can point to directories
77+
or files. If the path is a directory, it processes every `.dart` file in that
78+
directory and all of its subdirectories.
5779

58-
By default, it formats each file and write the formatting changes to the files.
59-
If you pass `--output show`, it prints the formatted code to stdout.
80+
By default, `dart format` formats each file and writes the result back to the
81+
same files. If you pass `--output show`, it prints the formatted code to stdout
82+
and doesn't modify the files.
6083

61-
You may pass a `-l` option to control the width of the page that it wraps lines
62-
to fit within, but you're strongly encouraged to keep the default line length of
63-
80 columns.
64-
65-
### Validating files
84+
## Validating formatting
6685

6786
If you want to use the formatter in something like a [presubmit script][] or
6887
[commit hook][], you can pass flags to omit writing formatting changes to disk
6988
and to update the exit code to indicate success/failure:
7089

71-
$ dart format --output=none --set-exit-if-changed .
90+
```sh
91+
$ dart format --output=none --set-exit-if-changed .
92+
```
7293

7394
[presubmit script]: https://www.chromium.org/developers/how-tos/depottools/presubmit-scripts
7495
[commit hook]: https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks
7596

76-
## Running other versions of the formatter CLI command
77-
78-
If you need to run a different version of the formatter, you can
79-
[globally activate][] the package from the dart_style package on
80-
pub.dev:
81-
82-
[globally activate]: https://dart.dev/tools/pub/cmd/pub-global
83-
84-
$ pub global activate dart_style
85-
$ pub global run dart_style:format ...
97+
## Using the formatter as a library
8698

87-
## Using the dart_style API
99+
The `dart_style package exposes a simple [library API][] for formatting code.
100+
Basic usage looks like this:
88101

89-
The package also exposes a single dart_style library containing a programmatic
90-
API for formatting code. Simple usage looks like this:
102+
[library api]: https://pub.dev/documentation/dart_style/latest/
91103

92104
```dart
93105
import 'package:dart_style/dart_style.dart';

lib/src/dart_formatter.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ import 'string_compare.dart' as string_compare;
2525
/// // dart format width=123
2626
final RegExp _widthCommentPattern = RegExp(r'^// dart format width=(\d+)$');
2727

28-
/// Dart source code formatter.
28+
/// A Dart source code formatter.
29+
///
30+
/// This is a lightweight class that mostly bundles formatting options so that
31+
/// you don't have to pass a long argument list to [format()] and
32+
/// [formatStatement()]. You can efficiently create a new instance of this for
33+
/// every format invocation.
2934
final class DartFormatter {
3035
/// The latest Dart language version that can be parsed and formatted by this
3136
/// version of the formatter.

lib/src/front_end/piece_factory.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ mixin PieceFactory {
449449
// ]) {
450450
// body;
451451
// }
452-
// TODO(tall): Passing `canBlockSplitLeft: true` allows output like:
452+
// TODO(rnystrom): Passing `canBlockSplitLeft: true` allows output like:
453453
//
454454
// // 1
455455
// for (variable in longExpression +
@@ -900,7 +900,7 @@ mixin PieceFactory {
900900

901901
/// Writes a [Piece] for an index expression.
902902
void writeIndexExpression(IndexExpression index) {
903-
// TODO(tall): Consider whether we should allow splitting between
903+
// TODO(rnystrom): Consider whether we should allow splitting between
904904
// successive index expressions, like:
905905
//
906906
// jsonData['some long key']

lib/src/front_end/piece_writer.dart

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ final class PieceWriter {
9696

9797
if (spaceBefore) space();
9898

99-
// TODO(perf): If [_currentCode] is `null` but [_pendingSpace] is `true`,
100-
// it should be possible to create a new code piece and write the leading
101-
// space to it instead of having a leading SpacePiece. Unfortunately, that
102-
// sometimes leads to duplicate spaces in the output, so it might take some
103-
// tweaking to get working.
99+
// TODO(rnystrom): If [_currentCode] is `null` but [_pendingSpace] is
100+
// `true`, it should be possible to create a new code piece and write the
101+
// leading space to it instead of having a leading SpacePiece.
102+
// Unfortunately, that sometimes leads to duplicate spaces in the output,
103+
// so it might take some tweaking to get working.
104104

105105
if (token.precedingComments != null) {
106106
// Don't append to the previous token if there is a comment after it.
@@ -269,11 +269,6 @@ final class PieceWriter {
269269
}
270270
}
271271

272-
// TODO(tall): Much of the comment handling code in CommentWriter got moved
273-
// into here, so there isn't great separation of concerns anymore. Can we
274-
// organize this code better? Or just combine CommentWriter with this class
275-
// completely?
276-
277272
/// Creates a new [Piece] for [comment] and returns it.
278273
Piece commentPiece(SourceComment comment,
279274
[Whitespace trailingWhitespace = Whitespace.none]) {

lib/src/piece/chain.dart

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -53,26 +53,6 @@ final class ChainPiece extends Piece {
5353
/// Allow newlines in the last (or next-to-last) call but nowhere else.
5454
static const State _blockFormatTrailingCall = State(1, cost: 0);
5555

56-
// TODO(tall): Currently, we only allow a single call in the chain to be
57-
// block-formatted, and it must be the last or next-to-last. That covers
58-
// the majority of common use cases (>90% of Flutter call chains), but there
59-
// are some cases (<1%) where it might be good to support multiple block
60-
// calls in a chain, like:
61-
//
62-
// future.then((_) {
63-
// doStuff();
64-
// }).then((_) {
65-
// moreStuff();
66-
// }).catchError((error) {
67-
// print('Oh no!');
68-
// });
69-
//
70-
// Decide if we want to support this and, if so, which calls are allowed to
71-
// be block formatted. A reasonable approach would be to say that multiple
72-
// block calls are allowed when the chain is (possibly zero) leading
73-
// properties followed by only splittable calls and all splittable calls get
74-
// block formatted.
75-
7656
/// Split the call chain at each method call, but leave the leading properties
7757
/// on the same line as the target.
7858
static const State _splitAfterProperties = State(2);

lib/src/piece/control_flow.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ final class ControlFlowPiece extends Piece {
6666
writer.splitIf(state == State.split);
6767
}
6868

69-
// TODO(perf): Investigate whether it's worth using `separate:` here.
69+
// TODO(rnystrom): Investigate whether it's worth using `separate:` here.
7070
writer.format(section.statement);
7171

7272
// Reset the indentation for the subsequent `else` or `} else` line.

lib/src/piece/piece.dart

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,6 @@ abstract base class Piece {
136136
///
137137
/// This is usually just the state's cost, but some pieces may want to tweak
138138
/// the cost in certain circumstances.
139-
// TODO(tall): Given that we have this API now, consider whether it makes
140-
// sense to remove the cost field from State entirely.
141139
int stateCost(State state) => state.cost;
142140

143141
/// Forces this piece to always use [state].

lib/src/piece/variable.dart

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ final class VariablePiece extends Piece {
8383
// Split between variables.
8484
if (i > 0) writer.splitIf(state != State.unsplit);
8585

86-
// TODO(perf): Investigate whether it's worth using `separate:` here.
8786
writer.format(_variables[i]);
8887
}
8988

lib/src/string_compare.dart

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@
22
// for details. All rights reserved. Use of this source code is governed by a
33
// BSD-style license that can be found in the LICENSE file.
44

5-
// TODO(tall): Now that the formatter may add and remove trailing commas and
6-
// reposition comments relative to `,`, `[`, `]`, `{`, and `}`, this function
7-
// is getting less and less precise. (For example, a bug in the formatter that
8-
// dropped all `[` tokens on the floor would still pass.) Consider a more
9-
// sophisticated approach for determining that the formatter preserved all of
10-
// the original code.
5+
// TODO(rnystrom): Now that the formatter may add and remove trailing commas
6+
// and reposition comments relative to `,`, `[`, `]`, `{`, and `}`, this
7+
// function is getting less and less precise. (For example, a bug in the
8+
// formatter that dropped all `[` tokens on the floor would still pass.)
9+
// Consider a more sophisticated approach for determining that the formatter
10+
// preserved all of the original code.
1111

1212
/// Returns `true` if [c] represents a whitespace code unit allowed in Dart
1313
/// source code.

0 commit comments

Comments
 (0)