Skip to content

Commit 35a26b2

Browse files
munificentmunificent
authored
Allow non-contiguous state values. (#1279)
Allow non-contiguous state values. Requiring the states to be a zero-based and contiguous makes it harder for some pieces to support different sets of different states based on what they contain. For example, an ImportPiece only supports states for splitting the names inside a single combinator when there is more than one combinator. To model that with contiguous states requires the piece to carefully track how state indexes are shifted or splitting pieces into different classes to handle the different sets of states. Instead, with this change, we simply allow the piece to return a list of states that may be non-contiguous. They can then omit states that don't make sense given the piece's contents. This makes ImportPiece simpler and I think will be useful for other pieces that have a variety of ways they can split. Also wrap state values in a class to make things a little clearer and more type safe.
1 parent 8f52f1e commit 35a26b2

File tree

11 files changed

+180
-179
lines changed

11 files changed

+180
-179
lines changed

‎lib/src/back_end/code_writer.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ class CodeWriter {
176176

177177
// TODO(tall): Support pieces with different split costs, and possibly
178178
// different costs for each state value.
179-
if (state != 0) _cost++;
179+
if (state != State.initial) _cost++;
180180

181181
// TODO(perf): Memoize this. Might want to create a nested PieceWriter
182182
// instead of passing in `this` so we can better control what state needs

‎lib/src/back_end/solution.dart

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class PieceStateSet {
1717
/// there's nothing to solve for them.
1818
final List<Piece> _pieces;
1919

20-
final Map<Piece, int> _pieceStates;
20+
final Map<Piece, State> _pieceStates;
2121

2222
/// Creates a new [PieceStateSet] with no pieces set to any state (which
2323
/// implicitly means they have state 0).
@@ -26,7 +26,7 @@ class PieceStateSet {
2626
PieceStateSet._(this._pieces, this._pieceStates);
2727

2828
/// The state this solution selects for [piece].
29-
int pieceState(Piece piece) => _pieceStates[piece] ?? 0;
29+
State pieceState(Piece piece) => _pieceStates[piece] ?? State.initial;
3030

3131
/// Gets the first piece that doesn't have a state selected yet, or `null` if
3232
/// all pieces have selected states.
@@ -42,7 +42,7 @@ class PieceStateSet {
4242
}
4343

4444
/// Creates a clone of this state with [piece] bound to [state].
45-
PieceStateSet cloneWith(Piece piece, int state) {
45+
PieceStateSet cloneWith(Piece piece, State state) {
4646
return PieceStateSet._(_pieces, {..._pieceStates, piece: state});
4747
}
4848

@@ -85,13 +85,13 @@ class Solution implements Comparable<Solution> {
8585
var piece = _state.firstUnsolved();
8686
if (piece == null) return const [];
8787

88-
var result = <Solution>[];
89-
for (var i = 0; i < piece.stateCount; i++) {
90-
var solution = Solution(root, pageWidth, _state.cloneWith(piece, i));
91-
result.add(solution);
92-
}
88+
return [
89+
// All pieces support a default state.
90+
Solution(root, pageWidth, _state.cloneWith(piece, State.initial)),
9391

94-
return result;
92+
for (var state in piece.states)
93+
Solution(root, pageWidth, _state.cloneWith(piece, state))
94+
];
9595
}
9696

9797
/// Compares two solutions where a more desirable solution comes first.

‎lib/src/back_end/solver.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class Solver {
3939

4040
void traverse(Piece piece) {
4141
// We don't need to worry about selecting pieces that have only one state.
42-
if (piece.stateCount > 1) unsolvedPieces.add(piece);
42+
if (piece.states.isNotEmpty) unsolvedPieces.add(piece);
4343
piece.forEachChild(traverse);
4444
}
4545

‎lib/src/front_end/piece_factory.dart

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,17 +145,10 @@ mixin PieceFactory implements CommentWriter {
145145
}
146146
}
147147

148-
var combinator = switch (combinators.length) {
149-
0 => null,
150-
1 => OneCombinatorPiece(combinators[0]),
151-
2 => TwoCombinatorPiece(combinators),
152-
_ => throw StateError('Directives can only have up to two combinators.'),
153-
};
154-
155148
token(directive.semicolon);
156149

157-
writer.push(
158-
ImportPiece(directivePiece, configurationsPiece, asClause, combinator));
150+
writer.push(ImportPiece(
151+
directivePiece, configurationsPiece, asClause, combinators));
159152
}
160153

161154
/// Creates a single infix operation.

‎lib/src/piece/block.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,13 @@ class BlockPiece extends Piece {
3030
: _alwaysSplit = alwaysSplit;
3131

3232
@override
33-
int get stateCount => _alwaysSplit ? 1 : 2;
33+
List<State> get states => _alwaysSplit ? const [] : const [State.split];
3434

3535
@override
36-
void format(CodeWriter writer, int state) {
36+
void format(CodeWriter writer, State state) {
3737
writer.format(leftBracket);
3838

39-
if (_alwaysSplit || state == 1) {
39+
if (_alwaysSplit || state == State.split) {
4040
writer.setIndent(Indent.block);
4141
writer.newline();
4242
writer.format(contents);

‎lib/src/piece/import.dart

Lines changed: 116 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -6,169 +6,150 @@ import '../back_end/code_writer.dart';
66
import '../constants.dart';
77
import 'piece.dart';
88

9-
/// An import or export directive.
9+
/// An import or export directive and its `show` and `hide` combinators.
1010
///
1111
/// Contains pieces for the keyword and URI, the optional `as` clause for
12-
/// imports, the configurations (`if` clauses), and combinators (`show` and
13-
/// `hide`).
12+
/// imports, and the configurations (`if` clauses).
13+
///
14+
/// Combinators can be split like so:
15+
///
16+
/// [State.initial] All on one line:
17+
///
18+
/// ```
19+
/// import 'animals.dart' show Ant, Bat hide Cat, Dog;
20+
/// ```
21+
///
22+
/// [_beforeCombinators] Wrap before each keyword:
23+
///
24+
/// ```
25+
/// import 'animals.dart'
26+
/// show Ant, Bat
27+
/// hide Cat, Dog;
28+
/// ```
29+
///
30+
/// [_firstCombinator] Wrap before each keyword and split the first list of
31+
/// names (only used when there are multiple combinators):
32+
///
33+
/// ```
34+
/// import 'animals.dart'
35+
/// show
36+
/// Ant,
37+
/// Bat
38+
/// hide Cat, Dog;
39+
/// ```
40+
///
41+
/// [_secondCombinator]: Wrap before each keyword and split the second list of
42+
/// names (only used when there are multiple combinators):
43+
///
44+
/// ```
45+
/// import 'animals.dart'
46+
/// show Ant, Bat
47+
/// hide
48+
/// Cat,
49+
/// Dog;
50+
/// ```
51+
///
52+
/// [State.split] Wrap before each keyword and split both lists of names:
53+
///
54+
/// ```
55+
/// import 'animals.dart'
56+
/// show
57+
/// Ant,
58+
/// Bat
59+
/// hide
60+
/// Cat,
61+
/// Dog;
62+
/// ```
63+
///
64+
/// These are not allowed:
65+
///
66+
/// ```
67+
/// // Wrap list but not keyword:
68+
/// import 'animals.dart' show
69+
/// Ant,
70+
/// Bat
71+
/// hide Cat, Dog;
72+
///
73+
/// // Wrap one keyword but not both:
74+
/// import 'animals.dart'
75+
/// show Ant, Bat hide Cat, Dog;
76+
///
77+
/// import 'animals.dart' show Ant, Bat
78+
/// hide Cat, Dog;
79+
/// ```
80+
///
81+
/// This ensures that when any wrapping occurs, the keywords are always at the
82+
/// beginning of the line.
1483
class ImportPiece extends Piece {
84+
/// Split before combinator keywords.
85+
static const _beforeCombinators = State(1);
86+
87+
/// Split before each name in the first combinator.
88+
static const _firstCombinator = State(2);
89+
90+
/// Split before each name in the second combinator.
91+
static const _secondCombinator = State(3);
92+
1593
/// The main directive and its URI.
16-
final Piece directive;
94+
final Piece _directive;
1795

1896
/// If the directive has `if` configurations, this is them.
19-
final Piece? configurations;
97+
final Piece? _configurations;
2098

2199
/// The `as` clause for this directive.
22100
///
23101
/// Null if this is not an import or it has no library prefix.
24-
final Piece? asClause;
25-
26-
/// The piece for the `show` and/or `hide` combinators.
27-
final Piece? combinator;
28-
29-
ImportPiece(
30-
this.directive, this.configurations, this.asClause, this.combinator);
102+
final Piece? _asClause;
31103

32-
@override
33-
int get stateCount => 1;
104+
final List<ImportCombinator> _combinators;
34105

35-
@override
36-
void format(CodeWriter writer, int state) {
37-
writer.format(directive);
38-
writer.formatOptional(configurations);
39-
writer.formatOptional(asClause);
40-
writer.formatOptional(combinator);
106+
ImportPiece(this._directive, this._configurations, this._asClause,
107+
this._combinators) {
108+
assert(_combinators.length <= 2);
41109
}
42110

43111
@override
44-
void forEachChild(void Function(Piece piece) callback) {
45-
callback(directive);
46-
if (configurations case var configurations?) callback(configurations);
47-
if (asClause case var asClause?) callback(asClause);
48-
if (combinator case var combinator?) callback(combinator);
49-
}
112+
List<State> get states => [
113+
_beforeCombinators,
114+
if (_combinators.length > 1) ...[
115+
_firstCombinator,
116+
_secondCombinator,
117+
],
118+
State.split
119+
];
50120

51121
@override
52-
String toString() => 'Directive';
53-
}
54-
55-
/// The combinator on a directive with only one combinator. It can be split:
56-
///
57-
/// // 0: All on one line:
58-
/// import 'animals.dart' show Ant, Bat, Cat;
59-
///
60-
/// // 1: Split before the keyword:
61-
/// import 'animals.dart'
62-
/// show Ant, Bat, Cat;
63-
///
64-
/// // 2: Split before the keyword and each name:
65-
/// import 'animals.dart'
66-
/// show
67-
/// Ant,
68-
/// Bat,
69-
/// Cat;
70-
class OneCombinatorPiece extends Piece {
71-
final ImportCombinator combinator;
72-
73-
OneCombinatorPiece(this.combinator);
74-
75-
/// 0: No splits anywhere.
76-
/// 1: Split before combinator keyword.
77-
/// 2: Split before combinator keyword and before each name.
78-
@override
79-
int get stateCount => 3;
122+
void format(CodeWriter writer, State state) {
123+
writer.format(_directive);
124+
writer.formatOptional(_configurations);
125+
writer.formatOptional(_asClause);
126+
127+
if (_combinators.isNotEmpty) {
128+
_combinators[0].format(writer,
129+
splitKeyword: state != State.initial,
130+
splitNames: state == _firstCombinator || state == State.split);
131+
}
80132

81-
@override
82-
void format(CodeWriter writer, int state) {
83-
combinator.format(writer, splitKeyword: state != 0, splitNames: state == 2);
133+
if (_combinators.length > 1) {
134+
_combinators[1].format(writer,
135+
splitKeyword: state != State.initial,
136+
splitNames: state == _secondCombinator || state == State.split);
137+
}
84138
}
85139

86140
@override
87141
void forEachChild(void Function(Piece piece) callback) {
88-
combinator.forEachChild(callback);
89-
}
90-
91-
@override
92-
String toString() => '1Comb';
93-
}
94-
95-
/// The combinators on a directive with two combinators. It can be split:
96-
///
97-
/// // 0: All on one line:
98-
/// import 'animals.dart' show Ant, Bat hide Cat, Dog;
99-
///
100-
/// // 1: Wrap before each keyword:
101-
/// import 'animals.dart'
102-
/// show Ant, Bat
103-
/// hide Cat, Dog;
104-
///
105-
/// // 2: Wrap before each keyword and split the first list of names:
106-
/// import 'animals.dart'
107-
/// show
108-
/// Ant,
109-
/// Bat
110-
/// hide Cat, Dog;
111-
///
112-
/// // 3: Wrap before each keyword and split the second list of names:
113-
/// import 'animals.dart'
114-
/// show Ant, Bat
115-
/// hide
116-
/// Cat,
117-
/// Dog;
118-
///
119-
/// // 4: Wrap before each keyword and split both lists of names:
120-
/// import 'animals.dart'
121-
/// show
122-
/// Ant,
123-
/// Bat
124-
/// hide
125-
/// Cat,
126-
/// Dog;
127-
///
128-
/// These are not allowed:
129-
///
130-
/// // Wrap list but not keyword:
131-
/// import 'animals.dart' show
132-
/// Ant,
133-
/// Bat
134-
/// hide Cat, Dog;
135-
///
136-
/// // Wrap one keyword but not both:
137-
/// import 'animals.dart'
138-
/// show Ant, Bat hide Cat, Dog;
139-
///
140-
/// import 'animals.dart' show Ant, Bat
141-
/// hide Cat, Dog;
142-
///
143-
/// This ensures that when any wrapping occurs, the keywords are always at
144-
/// the beginning of the line.
145-
class TwoCombinatorPiece extends Piece {
146-
final List<ImportCombinator> combinators;
147-
148-
TwoCombinatorPiece(this.combinators);
149-
150-
@override
151-
int get stateCount => 5;
152-
153-
@override
154-
void format(CodeWriter writer, int state) {
155-
assert(combinators.length == 2);
142+
callback(_directive);
143+
if (_configurations case var configurations?) callback(configurations);
144+
if (_asClause case var asClause?) callback(asClause);
156145

157-
combinators[0].format(writer,
158-
splitKeyword: state != 0, splitNames: state == 2 || state == 4);
159-
combinators[1].format(writer,
160-
splitKeyword: state != 0, splitNames: state == 3 || state == 4);
161-
}
162-
163-
@override
164-
void forEachChild(void Function(Piece piece) callback) {
165-
for (var combinator in combinators) {
146+
for (var combinator in _combinators) {
166147
combinator.forEachChild(callback);
167148
}
168149
}
169150

170151
@override
171-
String toString() => '2Comb';
152+
String toString() => 'Import';
172153
}
173154

174155
/// A single `show` or `hide` combinator within an import or export directive.

0 commit comments

Comments
 (0)