From 4dad0d3d6910e4cb572df3a8ec75df078d83958b Mon Sep 17 00:00:00 2001 From: belav Date: Mon, 29 Mar 2021 11:14:48 -0500 Subject: [PATCH] GH-22 figuring out a (kind of ugly) solution to get new lines working in trivia. --- .../TestFiles/Comments/ClassComments.cst | 6 ++ .../Comments/LeadingAndTrailingComments.cst | 22 ------- .../TestFiles/Comments/_CommentsTests.cs | 5 -- .../ConstructorDeclarationComments.cst | 2 +- .../TestFiles/Directives/Directives.cst | 12 +++- .../TestFiles/Directives/Regions.cst | 5 -- .../TestFiles/Directives/_DirectivesTests.cs | 5 -- Src/CSharpier/DocPrinter.cs | 9 +-- Src/CSharpier/PrinterHelpers/SyntaxTrivia.cs | 57 +++++++------------ 9 files changed, 39 insertions(+), 84 deletions(-) delete mode 100644 Src/CSharpier.Tests/TestFiles/Comments/LeadingAndTrailingComments.cst delete mode 100644 Src/CSharpier.Tests/TestFiles/Directives/Regions.cst diff --git a/Src/CSharpier.Tests/TestFiles/Comments/ClassComments.cst b/Src/CSharpier.Tests/TestFiles/Comments/ClassComments.cst index d7a73bf63..9cdfb734d 100644 --- a/Src/CSharpier.Tests/TestFiles/Comments/ClassComments.cst +++ b/Src/CSharpier.Tests/TestFiles/Comments/ClassComments.cst @@ -1,3 +1,9 @@ +// -------------------------------------------------------------------------------------------------------------------- +// +// NewLineAfterThis +// +// -------------------------------------------------------------------------------------------------------------------- + // public class public class ClassName { } diff --git a/Src/CSharpier.Tests/TestFiles/Comments/LeadingAndTrailingComments.cst b/Src/CSharpier.Tests/TestFiles/Comments/LeadingAndTrailingComments.cst deleted file mode 100644 index 9a41cc9f0..000000000 --- a/Src/CSharpier.Tests/TestFiles/Comments/LeadingAndTrailingComments.cst +++ /dev/null @@ -1,22 +0,0 @@ -class ClassName -{ - void MethodName() - { - bool b2; // trailing - } - - void MethodName() - { - // leading comment - var xx = 1; // trailing comment - - var x = 0; - - var y = 0; // trailing comment - - // leading comment - bool b1; - - bool b2; // trailing comment - } -} diff --git a/Src/CSharpier.Tests/TestFiles/Comments/_CommentsTests.cs b/Src/CSharpier.Tests/TestFiles/Comments/_CommentsTests.cs index c8e0c84fe..bbc341d6c 100644 --- a/Src/CSharpier.Tests/TestFiles/Comments/_CommentsTests.cs +++ b/Src/CSharpier.Tests/TestFiles/Comments/_CommentsTests.cs @@ -11,11 +11,6 @@ public void ClassComments() this.RunTest("Comments", "ClassComments"); } [Test] - public void LeadingAndTrailingComments() - { - this.RunTest("Comments", "LeadingAndTrailingComments"); - } - [Test] public void MethodComments() { this.RunTest("Comments", "MethodComments"); diff --git a/Src/CSharpier.Tests/TestFiles/ConstructorDeclaration/ConstructorDeclarationComments.cst b/Src/CSharpier.Tests/TestFiles/ConstructorDeclaration/ConstructorDeclarationComments.cst index 404b659bd..f38bfbed6 100644 --- a/Src/CSharpier.Tests/TestFiles/ConstructorDeclaration/ConstructorDeclarationComments.cst +++ b/Src/CSharpier.Tests/TestFiles/ConstructorDeclaration/ConstructorDeclarationComments.cst @@ -5,7 +5,7 @@ public class BasicClass // leading with line public // trailing public - // leading identifier + // leading identifier BasicClass // trailing identifier (int x) { } // trailing comments1 diff --git a/Src/CSharpier.Tests/TestFiles/Directives/Directives.cst b/Src/CSharpier.Tests/TestFiles/Directives/Directives.cst index 18a809921..fe25e5c06 100644 --- a/Src/CSharpier.Tests/TestFiles/Directives/Directives.cst +++ b/Src/CSharpier.Tests/TestFiles/Directives/Directives.cst @@ -1,5 +1,6 @@ #error Error message -#warning Warning message +#warning Warning new line test + #pragma warning disable 414, 3021 #pragma warning restore 3021 #pragma checksum "file.txt" "{00000000-0000-0000-0000-000000000000}" "2453" @@ -36,3 +37,12 @@ namespace Namespace void MethodName() { } } } + +#region Region + + #region Nested +class ClassName { } + #endregion Nested + +#endregion + diff --git a/Src/CSharpier.Tests/TestFiles/Directives/Regions.cst b/Src/CSharpier.Tests/TestFiles/Directives/Regions.cst deleted file mode 100644 index 4a4c3953b..000000000 --- a/Src/CSharpier.Tests/TestFiles/Directives/Regions.cst +++ /dev/null @@ -1,5 +0,0 @@ -#region Region - #region Nested -class ClassName { } - #endregion Nested -#endregion diff --git a/Src/CSharpier.Tests/TestFiles/Directives/_DirectivesTests.cs b/Src/CSharpier.Tests/TestFiles/Directives/_DirectivesTests.cs index de8919708..1542d04f8 100644 --- a/Src/CSharpier.Tests/TestFiles/Directives/_DirectivesTests.cs +++ b/Src/CSharpier.Tests/TestFiles/Directives/_DirectivesTests.cs @@ -10,10 +10,5 @@ public void Directives() { this.RunTest("Directives", "Directives"); } - [Test] - public void Regions() - { - this.RunTest("Directives", "Regions"); - } } } diff --git a/Src/CSharpier/DocPrinter.cs b/Src/CSharpier/DocPrinter.cs index 121c0a122..f4254666f 100644 --- a/Src/CSharpier/DocPrinter.cs +++ b/Src/CSharpier/DocPrinter.cs @@ -16,7 +16,7 @@ private Indent MakeIndent(Indent indent, Options options) { return this.GenerateIndent( indent, - new IndentType("indent", 0), + newPart: new IndentType("indent", 0), options ); } @@ -405,10 +405,6 @@ void Push(Doc doc, PrintMode printMode, Indent indent) { if (output.Length > 0) { - // if we have to undo this, another option I was considering was modifying NamespaceDeclarationSyntax - // when joining things with HardLines there, if the first item in each thing was a LiteralLine, skip the HardLine - // the problem with that approach is we'd have to make that same change in other places, but maybe - // there aren't a ton of those places. Trim(output); if (newLine.Length == 2) { @@ -424,7 +420,6 @@ void Push(Doc doc, PrintMode printMode, Indent indent) output.Length -= 1; } } - output.Append(newLine); position = 0; } @@ -466,7 +461,7 @@ void Push(Doc doc, PrintMode printMode, Indent indent) } output.Append( - command.Indent.Value + leadingComment.Comment + newLine + command.Indent.Value + command.Indent.Value + leadingComment.Comment ); position = command.Indent.Length; newLineNextStringValue = false; diff --git a/Src/CSharpier/PrinterHelpers/SyntaxTrivia.cs b/Src/CSharpier/PrinterHelpers/SyntaxTrivia.cs index 375ceac9a..f64dab69b 100644 --- a/Src/CSharpier/PrinterHelpers/SyntaxTrivia.cs +++ b/Src/CSharpier/PrinterHelpers/SyntaxTrivia.cs @@ -65,49 +65,39 @@ private Doc PrintLeadingTrivia(SyntaxToken syntaxToken) return this.PrintLeadingTrivia(syntaxToken.LeadingTrivia); } - // TODO 1 probably ditch this, but leave it around for now - private readonly Stack printNewLinesInLeadingTrivia = new(); - private Doc PrintLeadingTrivia(SyntaxTriviaList leadingTrivia) { var parts = new Parts(); - this.printNewLinesInLeadingTrivia.TryPeek(out var doNewLines); + // we don't print any new lines until we run into a comment or directive, the PrintNewLines method takes care of printing the initial new lines for a given node + // we also print double HardLines in some cases with directives. That's because a LiteralLine currently trims the previous new line it finds + // If it doesn't, we end up adding extra lines in some situations + // namespace Namespace + // { - HardLine - if we didn't trim this HardLine, then we'd end up inserting a blank line between this and #pragma + // #pragma - LiteralLine, #pragma + // vs + // #region Region - LiteralLine, #region, HardLine - we end each directive with a hardline to ensure we get a double hardline in this situation + // - HardLine - so the literal line trims this hardline, but we retain the newline between the region directives + // #region Nested - LiteralLine, #region, HardLine - because of the double hardline. + var printNewLines = false; - var hadDirective = false; for (var x = 0; x < leadingTrivia.Count; x++) { var trivia = leadingTrivia[x]; - if (doNewLines && trivia.Kind() == SyntaxKind.EndOfLineTrivia) + if ( + printNewLines + && trivia.Kind() == SyntaxKind.EndOfLineTrivia + ) { - SyntaxKind? kind = null; - if (x < leadingTrivia.Count - 1) - { - kind = leadingTrivia[x + 1].Kind(); - } - // TODO 0 this may screw up with regions that aren't at the beginning of the line? should we deal with new lines/trivia between things differently?? - if ( - !kind.HasValue - || kind == SyntaxKind.SingleLineCommentTrivia - || kind == SyntaxKind.EndOfLineTrivia - || kind == SyntaxKind.WhitespaceTrivia - ) - { - parts.Push(HardLine); - } + parts.Push(HardLine); } if ( trivia.Kind() != SyntaxKind.EndOfLineTrivia && trivia.Kind() != SyntaxKind.WhitespaceTrivia ) { - if (doNewLines) - { - doNewLines = false; - this.printNewLinesInLeadingTrivia.Pop(); - this.printNewLinesInLeadingTrivia.Push(false); - } + printNewLines = true; } if ( trivia.Kind() == SyntaxKind.SingleLineCommentTrivia @@ -155,10 +145,7 @@ private Doc PrintLeadingTrivia(SyntaxTriviaList leadingTrivia) || trivia.Kind() == SyntaxKind.NullableDirectiveTrivia ) { - hadDirective = true; - // GH-22 the problem is that we have a hardline followed by a literalline. What we really mean by literalLine is, add a line and dedent if this isn't preceded by a hardline.... I think - // we should probably figure out better new lines for directives, because that could affect this. - parts.Push(LiteralLine, trivia.ToString()); + parts.Push(LiteralLine, trivia.ToString(), HardLine); } else if ( trivia.Kind() == SyntaxKind.RegionDirectiveTrivia @@ -176,16 +163,10 @@ private Doc PrintLeadingTrivia(SyntaxTriviaList leadingTrivia) triviaText = leadingTrivia[x - 1] + triviaText; } - hadDirective = true; - parts.Push(LiteralLine, triviaText); + parts.Push(LiteralLine, triviaText, HardLine); } } - if (hadDirective) - { - parts.Push(HardLine); - } - return parts.Count > 0 ? Concat(parts) : Doc.Null; }