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

feat: add wrap_comments and comment_width options to nargo fmt #7371

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
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
93 changes: 76 additions & 17 deletions tooling/nargo_fmt/src/chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub(crate) enum Chunk {
/// (for example for a call we'll add a trailing comma to the last argument).
TrailingComma,
/// A trailing comment (happens at the end of a line, and always after something else have been written).
TrailingComment(TextChunk),
TrailingComment(TextChunk, bool /* at_block_end */),
/// A leading comment. Happens at the beginning of a line.
LeadingComment(TextChunk),
/// A group of chunks.
Expand All @@ -67,7 +67,7 @@ impl Chunk {
match self {
Chunk::Text(chunk)
| Chunk::Verbatim(chunk)
| Chunk::TrailingComment(chunk)
| Chunk::TrailingComment(chunk, _)
| Chunk::LeadingComment(chunk) => chunk.width,
Chunk::Group(group) => group.width(),
Chunk::SpaceOrLine => 1,
Expand Down Expand Up @@ -98,7 +98,7 @@ impl Chunk {
match self {
Chunk::Text(chunk)
| Chunk::Verbatim(chunk)
| Chunk::TrailingComment(chunk)
| Chunk::TrailingComment(chunk, _)
| Chunk::LeadingComment(chunk) => chunk.has_newlines,
Chunk::Group(group) => group.has_newlines(),
Chunk::TrailingComma
Expand Down Expand Up @@ -247,7 +247,15 @@ impl ChunkGroup {
/// Appends a trailing comment (it's formatted slightly differently than a regular text chunk).
pub(crate) fn trailing_comment(&mut self, chunk: TextChunk) {
if chunk.width > 0 {
self.push(Chunk::TrailingComment(chunk));
self.push(Chunk::TrailingComment(chunk, false));
}
}

/// Similar to `trailing_comment` but happens in a block end so no newline+indent will be
/// produced afterwards.
pub(crate) fn trailing_comment_at_block_end(&mut self, chunk: TextChunk) {
if chunk.width > 0 {
self.push(Chunk::TrailingComment(chunk, true));
}
}

Expand Down Expand Up @@ -370,7 +378,13 @@ impl ChunkGroup {
// so that it glues with the last text present there (if any)
group.add_trailing_comma_to_last_text();
}
Chunk::TrailingComment(chunk) => group.trailing_comment(chunk),
Chunk::TrailingComment(chunk, at_block_end) => {
if at_block_end {
group.trailing_comment_at_block_end(chunk);
} else {
group.trailing_comment(chunk);
}
}
Chunk::LeadingComment(chunk) => group.leading_comment(chunk),
Chunk::Group(inner_group) => group.group(inner_group),
Chunk::Line { two } => group.lines(two),
Expand Down Expand Up @@ -400,7 +414,7 @@ impl ChunkGroup {
match chunk {
Chunk::Text(text_chunk)
| Chunk::Verbatim(text_chunk)
| Chunk::TrailingComment(text_chunk)
| Chunk::TrailingComment(text_chunk, _)
| Chunk::LeadingComment(text_chunk) => {
width += text_chunk.width;
}
Expand Down Expand Up @@ -517,11 +531,14 @@ impl<'a, 'b> ChunkFormatter<'a, 'b> {
pub(crate) fn chunk(&mut self, f: impl FnOnce(&mut Formatter)) -> TextChunk {
let previous_buffer = std::mem::take(&mut self.0.buffer);
let previous_indentation = self.0.indentation;
let previous_in_chunk = self.0.in_chunk;
self.0.indentation = 0;
self.0.in_chunk = true;

f(self.0);

self.0.indentation = previous_indentation;
self.0.in_chunk = previous_in_chunk;

let buffer = std::mem::replace(&mut self.0.buffer, previous_buffer);
TextChunk::new(buffer.contents())
Expand Down Expand Up @@ -725,7 +742,7 @@ impl<'a> Formatter<'a> {
return;
}

// Check if the group first in the remainder of the current line.
// Check if the group fits in the remainder of the current line.
if total_width > self.max_width {
// If this chunk is the value of an assignment (either a normal assignment or a let statement)
// and it doesn't fit the current line, we check if it fits the next line with an increased
Expand Down Expand Up @@ -847,7 +864,10 @@ impl<'a> Formatter<'a> {
Chunk::Text(text_chunk) | Chunk::Verbatim(text_chunk) => {
self.write(&text_chunk.string);
}
Chunk::TrailingComment(text_chunk) | Chunk::LeadingComment(text_chunk) => {
Chunk::TrailingComment(text_chunk, _) => {
self.write(&text_chunk.string);
}
Chunk::LeadingComment(text_chunk) => {
self.write(&text_chunk.string);
self.write_space_without_skipping_whitespace_and_comments();
}
Expand Down Expand Up @@ -888,7 +908,7 @@ impl<'a> Formatter<'a> {
match chunk {
Chunk::Text(text_chunk) => {
if text_chunk.has_newlines {
self.write_chunk_lines(&text_chunk.string);
self.write_chunk_lines(&text_chunk.string, false);
} else {
// If we didn't exceed the max width, but this chunk will, insert a newline,
// increase indentation and indent (the indentation will be undone
Expand All @@ -909,16 +929,20 @@ impl<'a> Formatter<'a> {
Chunk::Verbatim(text_chunk) => {
self.write(&text_chunk.string);
}
Chunk::TrailingComment(text_chunk) => {
self.write_chunk_lines(&text_chunk.string);
self.write_line_without_skipping_whitespace_and_comments();
self.write_indentation();
Chunk::TrailingComment(text_chunk, at_block_end) => {
let is_comment = true;
self.write_chunk_lines(&text_chunk.string, is_comment);
if !at_block_end {
self.write_line_without_skipping_whitespace_and_comments();
self.write_indentation();
}
}
Chunk::LeadingComment(text_chunk) => {
let ends_with_multiple_newlines = text_chunk.string.ends_with("\n\n");
let ends_with_newline =
ends_with_multiple_newlines || text_chunk.string.ends_with('\n');
self.write_chunk_lines(text_chunk.string.trim());
let is_comment = true;
self.write_chunk_lines(text_chunk.string.trim(), is_comment);

// Respect whether the leading comment had a newline before what comes next or not
if ends_with_multiple_newlines {
Expand Down Expand Up @@ -1001,12 +1025,23 @@ impl<'a> Formatter<'a> {
}

/// Appends the string to the current buffer line by line, with some pre-checks.
fn write_chunk_lines(&mut self, string: &str) {
fn write_chunk_lines(&mut self, string: &str, is_comment: bool) {
// Here we also wrap comments if this chunk is a comment.
// The logic involves checking whether each line is part of a line or block comment
// and wrapping those accordingly.
let mut inside_block_comment = false;

let lines: Vec<_> = string.lines().collect();

let mut index = 0;
while index < lines.len() {
let line = &lines[index];
let mut line = lines[index];

let starts_with_space = line.starts_with(' ');
let is_line_comment =
is_comment && !inside_block_comment && line.trim_start().starts_with("//");
let is_block_comment =
is_comment && !inside_block_comment && line.trim_start().starts_with("/*");

// Don't indent the first line (it should already be indented).
// Also don't indent if the current line already has a space as the last char
Expand All @@ -1023,7 +1058,27 @@ impl<'a> Formatter<'a> {
// If we already have a space in the buffer and the line starts with a space,
// don't repeat that space.
if self.buffer.ends_with_space() && line.starts_with(' ') {
self.write(line.trim_start());
line = line.trim_start();
}

if is_line_comment && self.config.wrap_comments {
if starts_with_space && !self.buffer.ends_with_space() {
self.write(" ");
}
self.write_comment_with_prefix(
line.trim_start().strip_prefix("//").unwrap_or(line),
"//",
);
} else if (is_block_comment || inside_block_comment) && self.config.wrap_comments {
// Only append a space if it's the start of a block comment (no leading spaces in following lines)
if starts_with_space && is_block_comment && !self.buffer.ends_with_space() {
self.write(" ");
}
self.write_comment_with_prefix(line, "");
if inside_block_comment {
self.start_new_line();
}
inside_block_comment = true;
} else {
self.write(line);
}
Expand All @@ -1035,6 +1090,10 @@ impl<'a> Formatter<'a> {
self.write_multiple_lines_without_skipping_whitespace_and_comments();
index += 1;
}

if inside_block_comment && line.trim_end().ends_with("*/") {
inside_block_comment = false;
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions tooling/nargo_fmt/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ config! {
single_line_if_else_max_width: usize, 50, "Maximum line length for single line if-else expressions";
imports_granularity: ImportsGranularity, ImportsGranularity::Preserve, "How imports should be grouped into use statements.";
reorder_imports: bool, true, "Reorder imports alphabetically";
wrap_comments: bool, false, "Break comments to fit on the line";
comment_width: usize, 100, "Maximum length of comments. No effect unless `wrap_comments = true`"
}

impl Config {
Expand Down
13 changes: 5 additions & 8 deletions tooling/nargo_fmt/src/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,14 @@ pub(crate) struct Formatter<'a> {
pub(crate) group_tag_counter: usize,

/// We keep a copy of the config's max width because when we format chunk groups
/// we somethings change this so that a group has less space to write to.
/// we sometimes change this so that a group has less space to write to.
pub(crate) max_width: usize,

/// This is the buffer where we write the formatted code.
pub(crate) buffer: Buffer,

/// Is the formatter inside a chunk?
pub(crate) in_chunk: bool,
}

impl<'a> Formatter<'a> {
Expand All @@ -94,6 +97,7 @@ impl<'a> Formatter<'a> {
group_tag_counter: 0,
max_width: config.max_width,
buffer: Buffer::default(),
in_chunk: false,
};
formatter.bump();
formatter
Expand Down Expand Up @@ -206,13 +210,6 @@ impl<'a> Formatter<'a> {
self.bump();
}

/// Writes the current token trimming its end but doesn't advance to the next one.
/// Mainly used when writing comment lines, because we never want trailing spaces
/// inside comments.
pub(crate) fn write_current_token_trimming_end(&mut self) {
self.write(self.token.to_string().trim_end());
}

/// Writes the current token but without turning it into a string using `to_string()`.
/// Instead, we check the token's span and format what's in the original source there
/// (useful when formatting integer tokens, because a token like 0xFF ends up being an
Expand Down
Loading
Loading