Skip to content

Commit 222f670

Browse files
authored
Remove a bunch of TODO(tall) and TODO(perf) comments that aren't needed. (#1484)
These are all things we have either done already, have filed issues, or I don't have any plans to do at this point.
1 parent e49a9f3 commit 222f670

File tree

6 files changed

+5
-43
lines changed

6 files changed

+5
-43
lines changed

lib/src/back_end/code_writer.dart

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,11 @@ class CodeWriter {
118118
///
119119
/// If [text] contains any internal newlines, the caller is responsible for
120120
/// also calling [handleNewline()].
121+
///
122+
/// When possible, avoid calling this directly. Instead, any input code
123+
/// lexemes should be written to TextPieces which then call this. That way,
124+
/// selections inside lexemes are correctly updated.
121125
void write(String text) {
122-
// TODO(tall): Calling this directly from pieces outside of TextPiece may
123-
// not handle selections as gracefully as we could. A selection marker may
124-
// get pushed past the text written here. Currently, this is only called
125-
// directly for commas in list-like things, and `;` in for loops. In
126-
// general, it's better for all text written to the output to live inside
127-
// TextPieces because that will preserve selection markers. Consider doing
128-
// something smarter for commas in lists and semicolons in for loops.
129-
130126
_flushWhitespace();
131127
_buffer.write(text);
132128
_column += text.length;

lib/src/back_end/solution.dart

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -233,27 +233,11 @@ class Solution implements Comparable<Solution> {
233233
// end (or a winner).
234234
if (expandPiece == null) return const [];
235235

236-
// TODO(perf): If `_invalidPiece == expandPiece`, then we know that the
237-
// first state leads to an invalid solution, so there's no point in trying
238-
// to expand to a solution that binds `expandPiece` to
239-
// `expandPiece.states[0]`. We should be able to do:
240-
//
241-
// Iterable<State> states = expandPiece.states;
242-
// if (_invalidPiece == expandPiece) {
243-
// print('skip $expandPiece ${states.first}');
244-
// states = states.skip(1);
245-
// }
246-
//
247-
// And then use `states` below. But when I tried that, it didn't seem to
248-
// make any noticeable performance difference on the one pathological
249-
// example I tried. Leaving this here as a TODO to investigate more when
250-
// there are other benchmarks we can try.
251-
var solutions = <Solution>[];
252-
253236
// For each state that the expanding piece can be in, create a new solution
254237
// that inherits all of the bindings of this one, and binds the expanding
255238
// piece to that state (along with any further pieces constrained by that
256239
// one).
240+
var solutions = <Solution>[];
257241
for (var state in expandPiece.states) {
258242
var newStates = {..._pieceStates};
259243

lib/src/front_end/ast_node_visitor.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -523,9 +523,6 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
523523
if (node.members.isEmpty) {
524524
// If there are no members, format the constants like a delimited list.
525525
// This keeps the enum declaration on one line if it fits.
526-
// TODO(tall): The old style preserves blank lines and newlines between
527-
// enum values. A newline will also force the enum to split even if it
528-
// would otherwise fit. Do we want to do that with the new style too?
529526
var builder =
530527
DelimitedListBuilder(this, const ListStyle(spaceWhenUnsplit: true));
531528

lib/src/front_end/piece_writer.dart

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,6 @@ class PieceWriter {
302302
void _flushSpace() {
303303
if (!_pendingSpace) return;
304304

305-
// TODO(perf): See if we can make SpacePiece a constant to avoid creating
306-
// multiple.
307305
_pieces.last.add(SpacePiece());
308306
_pendingSpace = false;
309307
}

lib/src/piece/assign.dart

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -121,18 +121,6 @@ class AssignPiece extends Piece {
121121
_canBlockSplitLeft = canBlockSplitLeft,
122122
_canBlockSplitRight = canBlockSplitRight;
123123

124-
// TODO(tall): The old formatter allows the first operand of a split
125-
// conditional expression to be on the same line as the `=`, as in:
126-
//
127-
// var value = condition
128-
// ? thenValue
129-
// : elseValue;
130-
//
131-
// For now, we do not implement this special case behavior. Once more of the
132-
// language is implemented in the new back end and we can run the formatter
133-
// on a large corpus of code, we can try it out and see if the special case
134-
// behavior is worth it.
135-
136124
@override
137125
List<State> get additionalStates => [
138126
// If at least one operand can block split, allow splitting in operands

test/tall/invocation/chain_postfix.stmt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ someReceiverObject.property1.property2
4747
.property5
4848
.property6;
4949
<<<
50-
### TODO(tall): Allow splitting between successive indexes.
5150
someReceiverObject
5251
.property1
5352
.property2

0 commit comments

Comments
 (0)