Skip to content

Commit

Permalink
Only allow the comment at the top of the formatted code.
Browse files Browse the repository at this point in the history
  • Loading branch information
munificent committed Oct 5, 2024
1 parent 6d60708 commit 503da82
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 61 deletions.
20 changes: 19 additions & 1 deletion lib/src/dart_formatter.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ import 'short/source_visitor.dart';
import 'source_code.dart';
import 'string_compare.dart' as string_compare;

/// Regular expression that matches a format width comment like:
///
/// // dart format width=123
final RegExp _widthCommentPattern = RegExp(r'^// dart format width=(\d+)$');

/// Dart source code formatter.
class DartFormatter {
/// The latest Dart language version that can be parsed and formatted by this
Expand Down Expand Up @@ -178,8 +183,21 @@ class DartFormatter {

SourceCode output;
if (experimentFlags.contains(tallStyleExperimentFlag)) {
// Look for a page width comment before the code.
int? pageWidthFromComment;
for (Token? comment = node.beginToken.precedingComments;
comment != null;
comment = comment.next) {
if (_widthCommentPattern.firstMatch(comment.lexeme) case var match?) {
// If integer parsing fails for some reason, the returned `null`
// means we correctly ignore the comment.
pageWidthFromComment = int.tryParse(match[1]!);
break;
}
}

var visitor = AstNodeVisitor(this, lineInfo, unitSourceCode);
output = visitor.run(unitSourceCode, node);
output = visitor.run(unitSourceCode, node, pageWidthFromComment);
} else {
var visitor = SourceVisitor(this, lineInfo, unitSourceCode);
output = visitor.run(node);
Expand Down
7 changes: 5 additions & 2 deletions lib/src/front_end/ast_node_visitor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
///
/// This is the only method that should be called externally. Everything else
/// is effectively private.
SourceCode run(SourceCode source, AstNode node) {
///
/// If there is a `// dart format width=123` comment before the formatted
/// code, then [pageWidthFromComment] is that width.
SourceCode run(SourceCode source, AstNode node, int? pageWidthFromComment) {
Profile.begin('AstNodeVisitor.run()');

Profile.begin('AstNodeVisitor build Piece tree');
Expand Down Expand Up @@ -123,7 +126,7 @@ class AstNodeVisitor extends ThrowingAstVisitor<void> with PieceFactory {
Profile.end('AstNodeVisitor build Piece tree');

// Finish writing and return the complete result.
var result = pieces.finish(source, unitPiece);
var result = pieces.finish(source, unitPiece, pageWidthFromComment);

Profile.end('AstNodeVisitor.run()');

Expand Down
25 changes: 0 additions & 25 deletions lib/src/front_end/comment_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,29 +42,12 @@ import '../comment_type.dart';
/// written. When that [TextPiece] is output later, it will include the comments
/// as well.
class CommentWriter {
/// Regular expression that matches a format width comment like:
///
/// // dart format width=123
static final RegExp _widthCommentPattern =
RegExp(r'^// dart format width=(\d+)$');

final LineInfo _lineInfo;

/// The tokens whose preceding comments have already been taken by calls to
/// [takeCommentsBefore()].
final Set<Token> _takenTokens = {};

/// If we have encountered a comment that sets an explicit page width, that
/// width, otherwise `null`.
///
/// The comment looks like:
///
/// // dart format width=123
///
/// If there are multiple of these comments, the first one wins.
int? get pageWidthFromComment => _pageWidthFromComment;
int? _pageWidthFromComment;

CommentWriter(this._lineInfo);

/// Returns the comments that appear before [token].
Expand Down Expand Up @@ -123,14 +106,6 @@ class CommentWriter {
if (comment == token.precedingComments) linesBefore = 2;
}

// If this comment configures the page width, remember it.
if (_widthCommentPattern.firstMatch(text) case var match?
when _pageWidthFromComment == null) {
// If integer parsing fails for some reason, the returned `null`
// means we correctly ignore the comment.
_pageWidthFromComment = int.tryParse(match[1]!);
}

CommentType type;
if (text.startsWith('///') && !text.startsWith('////') ||
text.startsWith('/**') && text != '/**/') {
Expand Down
8 changes: 6 additions & 2 deletions lib/src/front_end/piece_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,11 @@ class PieceWriter {

/// Finishes writing and returns a [SourceCode] containing the final output
/// and updated selection, if any.
SourceCode finish(SourceCode source, Piece rootPiece) {
///
/// If there is a `// dart format width=123` comment before the formatted
/// code, then [pageWidthFromComment] is that width.
SourceCode finish(
SourceCode source, Piece rootPiece, int? pageWidthFromComment) {
if (debug.tracePieceBuilder) {
debug.log(debug.pieceTree(rootPiece));
}
Expand All @@ -399,7 +403,7 @@ class PieceWriter {

var cache = SolutionCache();
var solver = Solver(cache,
pageWidth: _comments.pageWidthFromComment ?? _formatter.pageWidth,
pageWidth: pageWidthFromComment ?? _formatter.pageWidth,
leadingIndent: _formatter.indent);
var solution = solver.format(rootPiece);
var output = solution.code.build(source, _formatter.lineEnding);
Expand Down
60 changes: 29 additions & 31 deletions test/tall/other/format_width.unit
Original file line number Diff line number Diff line change
@@ -1,118 +1,116 @@
40 columns |
### Tests for the comment to set formatting width.
>>> Comment sets page width.
// dart format width=30
main() {
// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30
main() {
// dart format width=30
fitsUnsplitAt40 +
butNotAt30;
}
>>> Comment anywhere affects all code.
>>> Comment only takes effect if it appears before code.
main() {
// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
// dart format width=30
<<<
main() {
fitsUnsplitAt40 +
butNotAt30;
// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}

>>> If there are multiple comments, first one wins.
// dart format width=30
>>> If multiple comments, first one wins.
// dart format width=60
main() {
// dart format width=30
// dart format width=60
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30
// dart format width=60
main() {
// dart format width=30
// dart format width=60
fitsUnsplitAt40 +
butNotAt30;
}
>>> Does nothing if width is not an integer.
// dart format width=wat
main() {
// dart format width=wat
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=wat
main() {
// dart format width=wat
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't have trailing text.
// dart format width=30 some more text
main() {
// dart format width=30 some more text
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30 some more text
main() {
// dart format width=30 some more text
fitsUnsplitAt40 + butNotAt30;
}
>>> Whitespace must match exactly.
//dart format width=30
main() {
//dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
<<<
//dart format width=30
main() {
//dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
>>> Whitespace must match exactly.
// dart format width=30
main() {
// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30
main() {
// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
>>> Whitespace must match exactly.
// dart format width = 30
main() {
// dart format width = 30
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width = 30
main() {
// dart format width = 30
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't be doc comment.
>>> Can't be a doc comment.
/// dart format width=30
main() {
/// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
<<<
/// dart format width=30
main() {
/// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't be nested inside other comment.
>>> Can't be nested inside another comment.
/* // dart format width=30 */
main() {
/* // dart format width=30 */
fitsUnsplitAt40 + butNotAt30;
}
<<<
/* // dart format width=30 */
main() {
/* // dart format width=30 */
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't be inside string literal.
>>> Can't be inside a string literal.
var c = '// dart format width=30';
main() {
var c = '// dart format width=30';
fitsUnsplitAt40 + butNotAt30;
}
<<<
var c = '// dart format width=30';
main() {
var c = '// dart format width=30';
fitsUnsplitAt40 + butNotAt30;
}

0 comments on commit 503da82

Please sign in to comment.