git.delta.rocks / jrsonnet / refs/commits / 443991edefaa

difftreelog

feat(fmt) preserve newline above last comment

Yaroslav Bolyukin2022-06-22parent: #dacee20.patch.diff
in: master

3 files changed

modifiedcmds/jrsonnet-fmt/src/children.rsdiffbeforeafterboth
--- a/cmds/jrsonnet-fmt/src/children.rs
+++ b/cmds/jrsonnet-fmt/src/children.rs
@@ -1,6 +1,6 @@
 // TODO: Return errors as trivia
 
-use std::{fmt::Debug, marker::PhantomData, mem};
+use std::{fmt::Debug, mem};
 
 use jrsonnet_rowan_parser::{
 	nodes::{Trivia, TriviaKind},
@@ -62,7 +62,7 @@
 	node: SyntaxNode,
 	start: Option<&SyntaxElement>,
 	end: Option<&SyntaxElement>,
-) -> ChildTrivia {
+) -> EndingComments {
 	let mut iter = node.children_with_tokens().peekable();
 	while iter.peek() != start {
 		iter.next();
@@ -85,14 +85,17 @@
 			)
 		}
 	}
-	out
+	EndingComments {
+		should_start_with_newline: should_start_with_newline(None, &out),
+		trivia: out,
+	}
 }
 
 pub fn children_between<T: AstNode + Debug>(
 	node: SyntaxNode,
 	start: Option<&SyntaxElement>,
 	end: Option<&SyntaxElement>,
-) -> (Vec<Child<T>>, ChildTrivia) {
+) -> (Vec<Child<T>>, EndingComments) {
 	let mut iter = node.children_with_tokens().peekable();
 	while iter.peek() != start {
 		iter.next();
@@ -104,9 +107,14 @@
 	)
 }
 
-pub fn should_start_with_newline(tt: &ChildTrivia) -> bool {
-	// First for previous item end
-	count_newlines_before(tt) >= 2
+pub fn should_start_with_newline(prev_inline: Option<&ChildTrivia>, tt: &ChildTrivia) -> bool {
+	count_newlines_before(tt)
+		+ prev_inline
+			.map(count_newlines_after)
+			.unwrap_or_default()
+
+		// First for previous item end, second for current item
+		>= 2
 }
 
 fn count_newlines_before(tt: &ChildTrivia) -> usize {
@@ -142,10 +150,10 @@
 	nl_count
 }
 
-pub fn children<'a, T: AstNode + Debug>(
+pub fn children<T: AstNode + Debug>(
 	items: impl Iterator<Item = SyntaxElement>,
 	loose: bool,
-) -> (Vec<Child<T>>, ChildTrivia) {
+) -> (Vec<Child<T>>, EndingComments) {
 	let mut out = Vec::new();
 	let mut current_child = None::<Child<T>>;
 	let mut next = ChildTrivia::new();
@@ -157,15 +165,12 @@
 		if let Some(value) = item.as_node().cloned().and_then(T::cast) {
 			let before_trivia = mem::take(&mut next);
 			let last_child = current_child.replace(Child {
-				newlines_above: if had_some {
-					count_newlines_before(&before_trivia)
-						+ current_child
-							.as_ref()
-							.map(|c| count_newlines_after(&c.inline_trivia))
-							.unwrap_or_default()
-				} else {
-					0
-				},
+				// First item should not start with newline
+				should_start_with_newline: had_some
+					&& should_start_with_newline(
+						current_child.as_ref().map(|c| &c.inline_trivia),
+						&before_trivia,
+					),
 				before_trivia,
 				value,
 				inline_trivia: Vec::new(),
@@ -206,16 +211,25 @@
 		}
 	}
 
+	let ending_comments = EndingComments {
+		should_start_with_newline: should_start_with_newline(
+			current_child.as_ref().map(|c| &c.inline_trivia),
+			&next,
+		),
+		trivia: next,
+	};
+
 	if let Some(current_child) = current_child {
 		out.push(current_child);
 	}
 
-	(out, next)
+	(out, ending_comments)
 }
 
 #[derive(Debug)]
 pub struct Child<T> {
-	newlines_above: usize,
+	/// If this child has two newlines above in source code, so it needs to have it in the output
+	pub should_start_with_newline: bool,
 	/// Comment before item, i.e
 	///
 	/// ```ignore
@@ -234,10 +248,8 @@
 	pub inline_trivia: ChildTrivia,
 }
 
-impl<T> Child<T> {
-	/// If this child has two newlines above in source code, so it needs to have it in output
-	pub fn needs_newline_above(&self) -> bool {
-		// First line for end of previous item
-		self.newlines_above >= 2
-	}
+pub struct EndingComments {
+	/// If this child has two newlines above in source code, so it needs to have it in the output
+	pub should_start_with_newline: bool,
+	pub trivia: ChildTrivia,
 }
modifiedcmds/jrsonnet-fmt/src/main.rsdiffbeforeafterboth
--- a/cmds/jrsonnet-fmt/src/main.rs
+++ b/cmds/jrsonnet-fmt/src/main.rs
@@ -13,7 +13,7 @@
 };
 
 use crate::{
-	children::{should_start_with_newline, trivia_after, trivia_between},
+	children::{trivia_after, trivia_between},
 	comments::{format_comments, CommentLocation},
 };
 
@@ -294,7 +294,7 @@
 					l.r_brace_token().map(Into::into).as_ref(),
 				);
 				for mem in children.into_iter() {
-					if mem.needs_newline_above() {
+					if mem.should_start_with_newline {
 						p!(pi: nl);
 					}
 					p!(pi: items(format_comments(&mem.before_trivia, CommentLocation::AboveItem)));
@@ -314,11 +314,10 @@
 					p!(pi: nl)
 				}
 
-				// TODO: implement same thing as needs_newline_above, but for end comments
-				if should_start_with_newline(&end_comments) {
+				if end_comments.should_start_with_newline {
 					p!(pi: nl);
 				}
-				p!(pi: items(format_comments(&end_comments, CommentLocation::EndOfItems)));
+				p!(pi: items(format_comments(&end_comments.trivia, CommentLocation::EndOfItems)));
 				p!(pi: <i str("}"));
 				pi
 			}
@@ -450,15 +449,17 @@
 				} else {
 					p!(pi: str("local") >i nl);
 					for bind in binds {
-						if bind.needs_newline_above() {
+						if bind.should_start_with_newline {
 							p!(pi: nl);
 						}
 						p!(pi: items(format_comments(&bind.before_trivia, CommentLocation::AboveItem)));
 						p!(pi: {bind.value} str(";"));
 						p!(pi: items(format_comments(&bind.inline_trivia, CommentLocation::ItemInline)) nl);
 					}
-					// TODO: needs_newline_above end_comments
-					p!(pi: items(format_comments(&end_comments, CommentLocation::EndOfItems)));
+					if end_comments.should_start_with_newline {
+						p!(pi: nl)
+					}
+					p!(pi: items(format_comments(&end_comments.trivia, CommentLocation::EndOfItems)));
 					p!(pi: <i);
 				}
 				p!(pi: str(";") nl);
@@ -471,9 +472,11 @@
 						.map(Into::into)
 						.as_ref(),
 				);
-				p!(pi: items(format_comments(&expr_comments, CommentLocation::AboveItem)));
 
-				// TODO: needs_newline_above expr
+				if expr_comments.should_start_with_newline {
+					p!(pi: nl);
+				}
+				p!(pi: items(format_comments(&expr_comments.trivia, CommentLocation::AboveItem)));
 				p!(pi: {l.expr()});
 				pi
 			}
modifiedcmds/jrsonnet-fmt/src/tests.rsdiffbeforeafterboth
before · cmds/jrsonnet-fmt/src/tests.rs
1use dprint_core::formatting::PrintOptions;2use indoc::indoc;34use crate::Printable;56fn reformat(input: &str) -> String {7	let (source, _) = jrsonnet_rowan_parser::parse(input);89	dprint_core::formatting::format(10		|| source.print(),11		PrintOptions {12			indent_width: 2,13			max_width: 100,14			use_tabs: false,15			new_line_text: "\n",16		},17	)18}1920macro_rules! assert_formatted {21	($input:literal, $output:literal) => {22		let formatted = reformat(indoc!($input));23		let expected = indoc!($output);24		if formatted != expected {25			panic!(26				"bad formatting, expected\n```\n{formatted}\n```\nto be equal to\n```\n{expected}\n```",27			)28		}29	};30}3132#[test]33fn padding_stripped_for_multiline_comment() {34	assert_formatted!(35		"{36            /*37                Hello38                    World39            */40            _: null,41        }",42		"{43          /*44          Hello45              World46          */47          _: null,48        }"49	);50}5152// Fails53#[test]54fn last_comment_respects_spacing_with_inline_comment_above() {55	assert_formatted!(56		"{57			a: '', // Inline5859			// Comment60        }",61		"{62		  a: '', // Inline6364		  // Comment65		}"66	);67}6869#[test]70fn complex_comments_snapshot() {71	insta::assert_display_snapshot!(reformat(indoc!(72		"{73		  comments: {74			_: '',75			//     Plain comment76			a: '',7778			#    Plain comment with empty line before79			b: '',80			/*Single-line multiline comment8182			*/83			c: '',8485			/**Single-line multiline doc comment8687			*/88			c: '',8990			/**Multiline doc91			Comment92			*/93			c: '',9495			/*9697	Multi-line9899	comment100			*/101			d: '',102103			e: '', // Inline comment104105			k: '',106107			// Text after everything108		  },109		  comments2: {110			k: '',111			// Text after everything, but no newline above112		  },113          spacing: {114            a: '',115116            b: '',117          },118          noSpacing: {119            a: '',120            b: '',121          },122        }"123	)))124}