Skip to content

Commit

Permalink
Improve comments in-between doc-comments and functions/methods
Browse files Browse the repository at this point in the history
This was discovered while adding tests for #2790, #2497. This is a
regression from #4185
  • Loading branch information
crodas committed Dec 5, 2023
1 parent 72e84a9 commit 5c9c734
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 6 deletions.
9 changes: 8 additions & 1 deletion swayfmt/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,14 @@ pub fn write_comments(
formatted_code.truncate(formatted_code.trim_end().len());
write!(formatted_code, " {} ", comment.span().as_str(),)?;
}
CommentKind::Multilined => {}
CommentKind::Multilined => {
write!(
formatted_code,
"{}{}",
formatter.indent_to_str()?,
comment.span().as_str(),
)?;
}
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions swayfmt/src/utils/language/attribute.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use crate::{
comments::write_comments,
constants::NEW_LINE,
formatter::*,
utils::{
map::byte_span::{ByteSpan, LeafSpans},
Expand All @@ -20,9 +22,21 @@ impl<T: Format + Spanned> Format for Annotated<T> {
formatter: &mut Formatter,
) -> Result<(), FormatterError> {
// format each `Attribute`
let mut start = None;
for attr in &self.attribute_list {
formatter.write_indent_into_buffer(formatted_code)?;
attr.format(formatted_code, formatter)?;
if start.is_none() {
start = Some(attr.span().end());
}
}
if let Some(start) = start {
// Write any comments that may have been defined in between the
// attributes and the value
write_comments(formatted_code, start..self.value.span().start(), formatter)?;
if !formatted_code.ends_with(NEW_LINE) {
write!(formatted_code, "{}", NEW_LINE)?;
}
}
// format `ItemKind`
formatter.write_indent_into_buffer(formatted_code)?;
Expand Down
32 changes: 27 additions & 5 deletions swayfmt/src/utils/map/newline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ fn newline_map_from_src(unformatted_input: &str) -> Result<NewlineMap, Formatter
let os_new_line = NEW_LINE.chars().collect::<Vec<_>>();
while let Some((char_index, char)) = input_iter.next() {
let next_char = input_iter.peek().map(|input| input.1);
if (char == '}' || char == ';') && is_new_line_next_in_iter(&mut input_iter, &os_new_line) {
if matches!(char, '/' | ';' | '}')
&& is_new_line_next_in_iter(&mut input_iter, &os_new_line)
{
if !in_sequence {
sequence_start = char_index + os_new_line.len();
in_sequence = true;
Expand Down Expand Up @@ -277,10 +279,13 @@ fn add_newlines(
}
}

let mut prev_character = None;
while let Some((_, character)) = whitespaces_with_comments_rev_it.next() {
if character == '/' {
// Comments either start with '//' or end with '*/'
if let Some((_, '/') | (_, '*')) = whitespaces_with_comments_rev_it.peek() {
let next_character =
whitespaces_with_comments_rev_it.peek().map(|(_, c)| *c);
if next_character == Some('/') || prev_character == Some('*') {
if let Some(newline_sequence) = first_newline_sequence_in_span(
&ByteSpan {
start: start + end_of_last_comment,
Expand All @@ -301,6 +306,7 @@ fn add_newlines(
break;
}
}
prev_character = Some(character);
}
}
}
Expand All @@ -318,6 +324,11 @@ fn format_newline_sequence(newline_sequence: &NewlineSequence, threshold: usize)
}
}

#[inline]
fn is_alphanumeric(c: char) -> bool {
c.is_alphanumeric() || c == '_' || c == '.'
}

/// Inserts a `NewlineSequence` at position `at` and returns the length of `NewlineSequence` inserted.
/// The return value is used to calculate the new `at` in a later point.
fn insert_after_span(
Expand Down Expand Up @@ -345,9 +356,20 @@ fn insert_after_span(
len -= (remove_until - at) as i64;
}

src_rope
.try_insert(at, &sequence_string)
.map_err(|_| FormatterError::NewlineSequenceError)?;
// Do never insert the newline sequence between two alphanumeric characters
if !src_rope
.get_char(at)
.map(is_alphanumeric)
.unwrap_or_default()
|| !src_rope
.get_char(at + 1)
.map(is_alphanumeric)
.unwrap_or_default()
{
src_rope
.try_insert(at, &sequence_string)
.map_err(|_| FormatterError::NewlineSequenceError)?;
}

formatted_code.clear();
formatted_code.push_str(&src_rope.to_string());
Expand Down
50 changes: 50 additions & 0 deletions swayfmt/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2263,3 +2263,53 @@ fn enum_in_vec() -> Vec<Pasta> {
"#,
);
}

#[test]
fn test_comment_random_places() {
check(
r#"library;
fn enum_in_vec() -> Vec<Pasta> {
let number: /*
this number is for counting
*/ u64 = 10;
let number: // this number is for counting
u64 = 10;
}
"#,
r#"library;
fn enum_in_vec() -> Vec<Pasta> {
let number: /*
this number is for counting
*/ u64 = 10;
let number: // this number is for counting
u64 = 10;
}
"#,
);
}

#[test]
fn test_comment_v2() {
check(
r#"library;
/// This is documentation for a commented out function
// fn commented_out_function() {
//}
fn test_function() -> bool {
true
}
"#,
r#"library;
/// This is documentation for a commented out function
// fn commented_out_function() {
//}
fn test_function() -> bool {
true
}
"#,
);
}

0 comments on commit 5c9c734

Please sign in to comment.