Skip to content
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

Ensure that line comments always get newlines after them. #1325

Merged
merged 2 commits into from
Nov 21, 2023
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
78 changes: 72 additions & 6 deletions lib/src/back_end/code_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,19 @@ class CodeWriter {
/// Buffer for the code being written.
final StringBuffer _buffer = StringBuffer();

/// What whitespace should be written before the next non-whitespace text.
///
/// When whitespace is written, instead of immediately writing it, we queue
/// it as pending. This ensures that we don't write trailing whitespace,
/// avoids writing spaces at the beginning of lines, and allows collapsing
/// multiple redundant newlines.
_Whitespace _pendingWhitespace = _Whitespace.none;

/// The number of spaces of indentation that should be begin the next line
/// when [_pendingWhitespace] is [_Whitespace.newline] or
/// [_Whitespace.blankLine].
int _pendingIndent = 0;

/// The cost of the currently chosen line splits.
int _cost = 0;

Expand Down Expand Up @@ -143,6 +156,7 @@ class CodeWriter {
// TextPieces because that will preserve selection markers. Consider doing
// something smarter for commas in lists and semicolons in for loops.

_flushWhitespace();
_buffer.write(text);
_column += text.length;

Expand Down Expand Up @@ -180,7 +194,10 @@ class CodeWriter {

/// Writes a single space to the output.
void space() {
write(' ');
// If a newline is already pending, then ignore the space.
if (_pendingWhitespace == _Whitespace.none) {
_pendingWhitespace = _Whitespace.space;
}
}

/// Inserts a line split in the output.
Expand All @@ -193,12 +210,15 @@ class CodeWriter {
if (indent != null) setIndent(indent);

handleNewline();
_finishLine();
_buffer.writeln();
if (blank) _buffer.writeln();

_column = _options.indent;
_buffer.write(' ' * _column);
// Collapse redundant newlines.
if (blank) {
_pendingWhitespace = _Whitespace.blankLine;
} else if (_pendingWhitespace != _Whitespace.blankLine) {
_pendingWhitespace = _Whitespace.newline;
}

_pendingIndent = _options.indent;
}

/// Sets whether newlines are allowed to occur from this point on for the
Expand Down Expand Up @@ -246,15 +266,46 @@ class CodeWriter {
/// Sets [selectionStart] to be [start] code units into the output.
void startSelection(int start) {
assert(_selectionStart == null);

_flushWhitespace();
_selectionStart = _buffer.length + start;
}

/// Sets [selectionEnd] to be [end] code units into the output.
void endSelection(int end) {
assert(_selectionEnd == null);

_flushWhitespace();
_selectionEnd = _buffer.length + end;
}

/// Write any pending whitespace.
///
/// This is called before non-whitespace text is about to be written, or
/// before the selection is updated since the latter requires an accurate
/// count of the written text, including whitespace.
void _flushWhitespace() {
switch (_pendingWhitespace) {
case _Whitespace.none:
break; // Nothing to do.

case _Whitespace.newline:
case _Whitespace.blankLine:
_finishLine();
_buffer.writeln();
if (_pendingWhitespace == _Whitespace.blankLine) _buffer.writeln();

_column = _pendingIndent;
_buffer.write(' ' * _column);

case _Whitespace.space:
_buffer.write(' ');
_column++;
}

_pendingWhitespace = _Whitespace.none;
}

void _finishLine() {
// If the completed line is too long, track the overflow.
if (_column >= _pageWidth) {
Expand All @@ -276,6 +327,21 @@ class CodeWriter {
}
}

/// Different kinds of pending whitespace that have been requested.
enum _Whitespace {
/// No pending whitespace.
none,

/// A single newline.
newline,

/// Two newlines.
blankLine,

/// A single space.
space
}

/// The mutable state local to a single piece being formatted.
class _PieceOptions {
/// The absolute number of spaces of leading indentation coming from
Expand Down
4 changes: 1 addition & 3 deletions lib/src/front_end/comment_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ mixin CommentWriter {

if (comments.isHanging(i)) {
// Attach the comment to the previous token.
pieces.writeSpace();
pieces.writeComment(comment, hanging: true);
} else {
pieces.writeNewline();
Expand Down Expand Up @@ -202,8 +201,7 @@ class SourceComment {
{required this.flushLeft, required this.offset});

/// Whether this comment contains a mandatory newline, either because it's a
/// comment that should be on its own line or a lexeme with a newline inside
/// it (i.e. multi-line block comment, multi-line string).
/// comment that should be on its own line or is a multi-line block comment.
bool get containsNewline =>
type != CommentType.inlineBlock || text.contains('\n');

Expand Down
16 changes: 5 additions & 11 deletions lib/src/front_end/piece_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,6 @@ class PieceWriter {
/// Whether we should write a space before the next text that is written.
bool _pendingSpace = false;

/// Whether we should write a newline in the current [TextPiece] before the
/// next text that is written.
bool _pendingNewline = false;

/// Whether we should create a new [TextPiece] the next time text is written.
bool _pendingSplit = false;

Expand Down Expand Up @@ -221,14 +217,14 @@ class PieceWriter {

/// Writes a mandatory newline from a comment to the current [TextPiece].
void writeNewline() {
_pendingNewline = true;
_currentText!.newline();
}

/// Write the contents of [comment] to the current innermost [TextPiece],
/// handling any newlines that may appear in it.
///
/// If [hanging] is `true`, then the comment is appended to the current line
/// even if call to [split()] has just happened. This is used for writing a
/// even if a call to [split()] has happened. This is used for writing a
/// comment that should be on the end of a line.
void writeComment(SourceComment comment, {bool hanging = false}) {
_write(comment.text,
Expand All @@ -246,10 +242,9 @@ class PieceWriter {
// current text.
if (textPiece == null || _pendingSplit && !hanging) {
textPiece = _currentText = TextPiece();
} else if (_pendingNewline) {
textPiece.newline();
} else if (_pendingSpace) {
textPiece.append(' ');
} else if (_pendingSpace || hanging) {
// Always write a space before hanging comments.
textPiece.appendSpace();
}

if (offset != null) {
Expand All @@ -267,7 +262,6 @@ class PieceWriter {
textPiece.append(text, containsNewline: containsNewline);

_pendingSpace = false;
_pendingNewline = false;
if (!hanging) _pendingSplit = false;
}

Expand Down
26 changes: 20 additions & 6 deletions lib/src/piece/piece.dart
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,19 @@ class TextPiece extends Piece {
/// multiline strings, etc.
bool _containsNewline = false;

/// Whether this piece should have a newline written at the end of it.
///
/// This is true during piece construction while lines are still being
/// written. It may also be true once a piece is fully complete if it ends in
/// a line comment.
bool _trailingNewline = false;

/// The offset from the beginning of [text] where the selection starts, or
/// `null` if the selection does not start within this chunk.
int? get selectionStart => _selectionStart;
int? _selectionStart;

/// The offset from the beginning of [text] where the selection ends, or
/// `null` if the selection does not start within this chunk.
int? get selectionEnd => _selectionEnd;
int? _selectionEnd;

/// Whether the last line of this piece's text ends with [text].
Expand All @@ -98,16 +103,25 @@ class TextPiece extends Piece {
/// If [text] internally contains a newline, then [containsNewline] should
/// be `true`.
void append(String text, {bool containsNewline = false}) {
if (_lines.isEmpty) _lines.add('');
if (_lines.isEmpty || _trailingNewline) _lines.add('');

// TODO(perf): Consider a faster way of accumulating text.
_lines.last = _lines.last + text;

if (containsNewline) _containsNewline = true;

_trailingNewline = false;
}

void appendSpace() {
// Don't write an unnecessary space at the beginning of a line.
if (_trailingNewline) return;

append(' ');
}

void newline() {
_lines.add('');
_trailingNewline = true;
}

@override
Expand All @@ -128,6 +142,8 @@ class TextPiece extends Piece {
if (i > 0) writer.newline();
writer.write(_lines[i]);
}

if (_trailingNewline) writer.newline();
}

@override
Expand All @@ -141,7 +157,6 @@ class TextPiece extends Piece {
start += line.length;
}

// print('TextPiece start $start (absolute = $abs)');
_selectionStart = start;
}

Expand All @@ -153,7 +168,6 @@ class TextPiece extends Piece {
end += line.length;
}

// print('TextPiece end $end (absolute = $abs)');
_selectionEnd = end;
}

Expand Down