Skip to content

Allow a comment in code to explicitly configure the page width. #1574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
9 changes: 7 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,8 @@ class PieceWriter {

var cache = SolutionCache();
var solver = Solver(cache,
pageWidth: _formatter.pageWidth, leadingIndent: _formatter.indent);
pageWidth: pageWidthFromComment ?? _formatter.pageWidth,
leadingIndent: _formatter.indent);
var solution = solver.format(rootPiece);
var output = solution.code.build(source, _formatter.lineEnding);

Expand Down
130 changes: 130 additions & 0 deletions test/tall/other/format_off.unit
Original file line number Diff line number Diff line change
Expand Up @@ -193,4 +193,134 @@ main() {
unformatted + code + // dart format on
after +
region;
}
>>> "dart format off" whitespace must match exactly.
main() {
//dart format off
unformatted + code;
// dart format on

// dart format off
unformatted + code;
// dart format on

// dart format off
unformatted + code;
// dart format on

// dart format off
unformatted + code;
// dart format on
}
<<<
main() {
//dart format off
unformatted + code;
// dart format on

// dart format off
unformatted + code;
// dart format on

// dart format off
unformatted + code;
// dart format on

// dart format off
unformatted + code;
// dart format on
}
>>> "dart format on" whitespace must match exactly.
main() {
// dart format off
// Doesn't actually turn back on:
//dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;

// dart format off
// Doesn't actually turn back on:
// dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;

// dart format off
// Doesn't actually turn back on:
// dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;

// dart format off
// Doesn't actually turn back on:
// dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;
}
<<<
main() {
// dart format off
// Doesn't actually turn back on:
//dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;

// dart format off
// Doesn't actually turn back on:
// dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;

// dart format off
// Doesn't actually turn back on:
// dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;

// dart format off
// Doesn't actually turn back on:
// dart format on
unformatted + code;

// Does now:
// dart format on
unformatted + code;
}
>>> Can't be doc comment.
main() {
here + gets + formatted ;
/// dart format off
here + does + too ;
/// dart format on
and + here + does ;
}
<<<
main() {
here + gets + formatted;

/// dart format off
here + does + too;

/// dart format on
and + here + does;
}
116 changes: 116 additions & 0 deletions test/tall/other/format_width.unit
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
40 columns |
### Tests for the comment to set formatting width.
>>> Comment sets page width.
// dart format width=30
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30
main() {
fitsUnsplitAt40 +
butNotAt30;
}
>>> Comment only takes effect if it appears before code.
main() {
// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
<<<
main() {
// dart format width=30
fitsUnsplitAt40 + butNotAt30;
}
>>> If there are multiple comments, first one wins.
// dart format width=30
// dart format width=60
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30
// dart format width=60
main() {
fitsUnsplitAt40 +
butNotAt30;
}
>>> Does nothing if width is not an integer.
// dart format width=wat
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=wat
main() {
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't have trailing text.
// dart format width=30 some more text
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30 some more text
main() {
fitsUnsplitAt40 + butNotAt30;
}
>>> Whitespace must match exactly.
//dart format width=30
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
//dart format width=30
main() {
fitsUnsplitAt40 + butNotAt30;
}
>>> Whitespace must match exactly.
// dart format width=30
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width=30
main() {
fitsUnsplitAt40 + butNotAt30;
}
>>> Whitespace must match exactly.
// dart format width = 30
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
// dart format width = 30
main() {
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't be a doc comment.
/// dart format width=30
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
/// dart format width=30
main() {
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't be nested inside another comment.
/* // dart format width=30 */
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
/* // dart format width=30 */
main() {
fitsUnsplitAt40 + butNotAt30;
}
>>> Can't be inside a string literal.
var c = '// dart format width=30';
main() {
fitsUnsplitAt40 + butNotAt30;
}
<<<
var c = '// dart format width=30';
main() {
fitsUnsplitAt40 + butNotAt30;
}