Skip to content

Commit

Permalink
Improving how we pretty print by trying to limit line length and avoi…
Browse files Browse the repository at this point in the history 
…d eagerly breaking lines (#13)

Co-authored-by: James Luck <[email protected]>
  • Loading branch information
@djluck
djluck and djluck authored Feb 13, 2023
1 parent 0dc0b89 commit 63898c1
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 41 deletions.
93 changes: 66 additions & 27 deletions src/PromQL.Parser/Printer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class Printer : IVisitor
public Printer(PrinterOptions? options = null)
{
_options = options ?? PrinterOptions.PrettyDefault;
_sb = new IndentedStringBuilder(_options.IndentChar, _options.IndentCount);
_sb = new IndentedStringBuilder(_options.IndentChar, _options.IndentCount, _options.BreakAfterCharCount);
}

public virtual void Visit(StringLiteral s)
Expand Down Expand Up @@ -146,7 +146,7 @@ public virtual void Visit(ParenExpression paren)
{
Write("(");

using (_options.BreakOnParenExpr ? _sb.IncreaseIndent() : null)
using (_options.BreakOnParenExpr ? _sb.StartScope() : null)
paren.Expr.Accept(this);

Write(")");
Expand Down Expand Up @@ -209,12 +209,11 @@ public virtual void Visit(VectorMatching vm)
public virtual void Visit(BinaryExpr expr)
{
expr.LeftHandSide.Accept(this);
Write(" ");

if (_options.BreakOnBinaryOperators)
_sb.AppendLine();
else
Write(" ");

_sb.AddPotentialBreakPoint();

Write($"{expr.Operator.ToPromQl()} ");

var preLen = _sb.Length;
Expand Down Expand Up @@ -259,56 +258,95 @@ public string ToPromQl(IPromQlNode node)
throw new ArgumentNullException();

_sb.Clear();
node.Accept(this);
using(_sb.StartScope())
node.Accept(this);

return _sb.ToString()!;
}
}

internal class IndentedStringBuilder
{
private int _indentLevel = 0;
private readonly string[] _indents;
private const int MaxIndent = 20;
private StringBuilder _sb = new StringBuilder();

public IndentedStringBuilder(char indentChar, int indentCount)
private Stack<Scope> _scopes = new Stack<Scope>();
private readonly int _idealLineLength;

public IndentedStringBuilder(char indentChar, int indentCount, int idealLineLength)
{
_idealLineLength = idealLineLength;
_indents = Enumerable.Range(0, MaxIndent)
.Select(n => new string(Enumerable.Repeat(indentChar, indentCount * n).ToArray()))
.Select(n => Environment.NewLine + new string(Enumerable.Repeat(indentChar, indentCount * n).ToArray()))
.ToArray();
}

public void Append(string? s) => _sb.Append(s);

public void Append(string? s)
{
CheckForScope();
_sb.Append(s);
}

public int Length => _sb.Length;

public void Clear() => _sb.Clear();

public void AppendLine()
private void AppendLine(int indent)
{
_sb.AppendLine();
_sb.Append(_indents[_indentLevel]);
_sb.Append(_indents[indent]);
}

public override string ToString() => _sb.ToString();

public IDisposable IncreaseIndent()
public IDisposable StartScope()
{
_indentLevel++;
if (_indentLevel == MaxIndent)
if (_scopes.Count == MaxIndent)
throw new InvalidOperationException($"Cannot increase indent beyond {MaxIndent}");

AppendLine();
_scopes.Push(new Scope(_sb.Length));

return new DisposableIndent(this);
}

public void DecreaseIndent()
public void AddPotentialBreakPoint() => _scopes.Peek().BreakPositions.Add(_sb.Length);

public void EndScope()
{
var indentLevel = _scopes.Count - 1;
var scope = _scopes.Pop();

if (_sb.Length - scope.StartIndex <= _idealLineLength)
return;

int insertedIndent = 0;

if (scope.StartIndex > 0)
{
_sb.Insert(scope.StartIndex, _indents[indentLevel]);
insertedIndent += _indents[indentLevel].Length;
}

foreach (var i in scope.BreakPositions)
{
_sb.Insert(i + insertedIndent, _indents[indentLevel]);
insertedIndent += _indents[indentLevel].Length;
}

if (indentLevel != 0)
AppendLine(indentLevel - 1);
}

private void CheckForScope()
{
_indentLevel--;
AppendLine();
if (_scopes.Count == 0)
throw new InvalidOperationException($"Must start a scope with {nameof(StartScope)}()");
}

public record struct Scope(int StartIndex)
{
public List<int> BreakPositions { get; } = new ();
};

internal class DisposableIndent : IDisposable
{
private readonly IndentedStringBuilder _isb;
Expand All @@ -325,7 +363,7 @@ public void Dispose()
if (_disposed)
return;

_isb.DecreaseIndent();
_isb.EndScope();
_disposed = true;
}
}
Expand All @@ -335,18 +373,19 @@ public record PrinterOptions(
int IndentCount,
char IndentChar,
bool BreakOnParenExpr,
bool BreakOnBinaryOperators
bool BreakOnBinaryOperators,
int BreakAfterCharCount
)
{
/// <summary>
/// Formats the printed output of PromQL expressions into a more human-readable format,
/// by inserting line breaks and indentation.
/// </summary>
public static PrinterOptions PrettyDefault = new(2, ' ', true, true);
public static PrinterOptions PrettyDefault = new(2, ' ', true, true, 100);

/// <summary>
/// Doesn't format the printed output of PromQL expressions.
/// </summary>
public static PrinterOptions NoFormatting = new(0, ' ', false, false);
public static PrinterOptions NoFormatting = new(0, ' ', false, false, int.MaxValue);
}
}
2 changes: 1 addition & 1 deletion src/PromQL.Parser/PromQL.Parser.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<TargetFrameworks>netcoreapp3.1;net6.0</TargetFrameworks>
<LangVersion>9</LangVersion>
<LangVersion>10</LangVersion>
<Nullable>enable</Nullable>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
<PackageId>PromQL.Parser</PackageId>
Expand Down
70 changes: 57 additions & 13 deletions tests/PromQL.Parser.Tests/PrinterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,30 +214,74 @@ public class PrettyPrintTests
private Printer _printer = new Printer(PrinterOptions.PrettyDefault);

[Test]
public void ParenExpression() => _printer.ToPromQl(new ParenExpression(new NumberLiteral(1.0)))
.Should().Be(@"(
1
public void ParenExpressionShort() => _printer.ToPromQl(new ParenExpression(new NumberLiteral(1.0)))
.Should().Be(@"(1)");

[Test]
public void ParenExpressionNestedShort() => _printer.ToPromQl(new ParenExpression(new ParenExpression(new NumberLiteral(1.0))))
.Should().Be(@"((1))");

[Test]
public void BinaryExpressionShort() => _printer.ToPromQl(new BinaryExpr(new NumberLiteral(1.0), new NumberLiteral(1.0), Operators.Binary.Add))
.Should().Be(@"1 + 1");

[Test]
public void ParenBinaryExpressionShort() => _printer.ToPromQl(new ParenExpression(new BinaryExpr(new NumberLiteral(1.0), new NumberLiteral(1.0), Operators.Binary.Add)))
.Should().Be(@"(1 + 1)");

[Test]
public void BinaryExpression() => _printer.ToPromQl(Parser.ParseExpression("sum(rate(this_is_a_long_metric_name{environment='production'}[5m])) + sum(rate(this_is_another_long_metric_name{environment='production'}[5m]))"))
.Should().Be(
@"sum(rate(this_is_a_long_metric_name{environment='production'}[5m]))
+ sum(rate(this_is_another_long_metric_name{environment='production'}[5m]))");

[Test]
public void BinaryWrappedParenExpression() => _printer.ToPromQl(Parser.ParseExpression("(sum(rate(this_is_a_long_metric_name{environment='production'}[5m])) + sum(rate(this_is_another_long_metric_name{environment='production'}[5m])))"))
.Should().Be(
@"(
sum(rate(this_is_a_long_metric_name{environment='production'}[5m]))
+ sum(rate(this_is_another_long_metric_name{environment='production'}[5m]))
)");

[Test]
public void ParenExpressionNested() => _printer.ToPromQl(new ParenExpression(new ParenExpression(new NumberLiteral(1.0))))
.Should().Be(@"(
public void BinaryWrappedParenExpression2() => _printer.ToPromQl(Parser.ParseExpression("((sum(rate(this_is_a_long_metric_name{environment='production'}[5m])) + sum(rate(this_is_another_long_metric_name{environment='production'}[5m]))))"))
.Should().Be(
@"(
(
1
sum(rate(this_is_a_long_metric_name{environment='production'}[5m]))
+ sum(rate(this_is_another_long_metric_name{environment='production'}[5m]))
)
)");

[Test]
public void BinaryExpression() => _printer.ToPromQl(new BinaryExpr(new NumberLiteral(1.0), new NumberLiteral(1.0), Operators.Binary.Add))
.Should().Be(@"1
+ 1");
public void BinaryExpression2() => _printer.ToPromQl(Parser.ParseExpression("1 + 2 + 3 + 4 + 5 + sum(rate(this_is_another_long_metric_name{environment='production', code=~'5.*'}[5m]))"))
.Should().Be(
@"1
+ 2
+ 3
+ 4
+ 5
+ sum(rate(this_is_another_long_metric_name{environment='production', code=~'5.*'}[5m]))");

[Test]
public void ParenBinaryExpression() => _printer.ToPromQl(new ParenExpression(new BinaryExpr(new NumberLiteral(1.0), new NumberLiteral(1.0), Operators.Binary.Add)))
public void BinaryExpressionNestedParenShort() => _printer.ToPromQl(Parser.ParseExpression("1 + 2 + (3 + (4 + 5))"))
.Should().Be(
@"1 + 2 + (3 + (4 + 5))");

[Test]
public void BinaryExpressionNestedParenLong() => _printer.ToPromQl(Parser.ParseExpression("1 + (2 + 3) + sum(rate(this_is_another_long_metric_name{environment='production', code=~'5.*', endpoint='/api/test'}[5m]))"))
.Should().Be(
@"1
+ (2 + 3)
+ sum(rate(this_is_another_long_metric_name{environment='production', code=~'5.*', endpoint='/api/test'}[5m]))");


[Test]
public void ParenBinaryExpression() => _printer.ToPromQl(Parser.ParseExpression("(sum(rate(this_is_a_long_metric_name{environment='production'}[5m])) + sum(rate(this_is_another_long_metric_name{environment='production'}[5m])))"))
.Should().Be(@"(
1
+ 1
sum(rate(this_is_a_long_metric_name{environment='production'}[5m]))
+ sum(rate(this_is_another_long_metric_name{environment='production'}[5m]))
)");

}
}

0 comments on commit 63898c1

Please sign in to comment.