From 4f6eca4fcbcb76a3c28a2004a1efbc32b6c99548 Mon Sep 17 00:00:00 2001 From: Patrik Svensson Date: Tue, 24 Nov 2020 22:03:31 +0100 Subject: [PATCH] Fix rendering of exceptions with generic params Closes #145 --- src/Spectre.Console.Tests/Data/Exceptions.cs | 15 +++++++++ ...eters_In_Callsite_As_Expected.verified.txt | 7 ++++ .../Unit/ExceptionTests.cs | 14 ++++++++ .../Extensions/StringBuilderExtensions.cs | 29 ++++++++++++++++ .../Internal/ExceptionFormatter.cs | 33 +++++++++++-------- 5 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 src/Spectre.Console.Tests/Expectations/ExceptionTests.Should_Write_Exceptions_With_Generic_Type_Parameters_In_Callsite_As_Expected.verified.txt create mode 100644 src/Spectre.Console/Extensions/StringBuilderExtensions.cs diff --git a/src/Spectre.Console.Tests/Data/Exceptions.cs b/src/Spectre.Console.Tests/Data/Exceptions.cs index e79c84b..7dfa4d9 100644 --- a/src/Spectre.Console.Tests/Data/Exceptions.cs +++ b/src/Spectre.Console.Tests/Data/Exceptions.cs @@ -8,6 +8,9 @@ namespace Spectre.Console.Tests.Data [SuppressMessage("Usage", "CA1801:Review unused parameters", Justification = "")] public static bool MethodThatThrows(int? number) => throw new InvalidOperationException("Throwing!"); + [SuppressMessage("Usage", "CA1801:Review unused parameters", Justification = "")] + public static bool GenericMethodThatThrows(int? number) => throw new InvalidOperationException("Throwing!"); + public static void ThrowWithInnerException() { try @@ -19,5 +22,17 @@ namespace Spectre.Console.Tests.Data throw new InvalidOperationException("Something threw!", ex); } } + + public static void ThrowWithGenericInnerException() + { + try + { + GenericMethodThatThrows(null); + } + catch (Exception ex) + { + throw new InvalidOperationException("Something threw!", ex); + } + } } } diff --git a/src/Spectre.Console.Tests/Expectations/ExceptionTests.Should_Write_Exceptions_With_Generic_Type_Parameters_In_Callsite_As_Expected.verified.txt b/src/Spectre.Console.Tests/Expectations/ExceptionTests.Should_Write_Exceptions_With_Generic_Type_Parameters_In_Callsite_As_Expected.verified.txt new file mode 100644 index 0000000..8a394f6 --- /dev/null +++ b/src/Spectre.Console.Tests/Expectations/ExceptionTests.Should_Write_Exceptions_With_Generic_Type_Parameters_In_Callsite_As_Expected.verified.txt @@ -0,0 +1,7 @@ +System.InvalidOperationException: Something threw! + System.InvalidOperationException: Throwing! + at Spectre.Console.Tests.Data.TestExceptions.GenericMethodThatThrows[[T0,T1,TRet]](Nullable`1 number) in /xyz/Exceptions.cs:nn + at Spectre.Console.Tests.Data.TestExceptions.ThrowWithGenericInnerException() in /xyz/Exceptions.cs:nn + at Spectre.Console.Tests.Data.TestExceptions.ThrowWithGenericInnerException() in /xyz/Exceptions.cs:nn + at Spectre.Console.Tests.Unit.ExceptionTests.<>c.b__4_0() in /xyz/ExceptionTests.cs:nn + at Spectre.Console.Tests.Unit.ExceptionTests.GetException(Action action) in /xyz/ExceptionTests.cs:nn diff --git a/src/Spectre.Console.Tests/Unit/ExceptionTests.cs b/src/Spectre.Console.Tests/Unit/ExceptionTests.cs index 5de58e8..b7a1748 100644 --- a/src/Spectre.Console.Tests/Unit/ExceptionTests.cs +++ b/src/Spectre.Console.Tests/Unit/ExceptionTests.cs @@ -65,6 +65,20 @@ namespace Spectre.Console.Tests.Unit return Verifier.Verify(result); } + [Fact] + public Task Should_Write_Exceptions_With_Generic_Type_Parameters_In_Callsite_As_Expected() + { + // Given + var console = new PlainConsole(width: 1024); + var dex = GetException(() => TestExceptions.ThrowWithGenericInnerException()); + + // When + var result = console.WriteNormalizedException(dex); + + // Then + return Verifier.Verify(result); + } + public static Exception GetException(Action action) { try diff --git a/src/Spectre.Console/Extensions/StringBuilderExtensions.cs b/src/Spectre.Console/Extensions/StringBuilderExtensions.cs new file mode 100644 index 0000000..ff394f6 --- /dev/null +++ b/src/Spectre.Console/Extensions/StringBuilderExtensions.cs @@ -0,0 +1,29 @@ +using System.Globalization; +using System.Text; + +namespace Spectre.Console +{ + internal static class StringBuilderExtensions + { + public static StringBuilder AppendWithStyle(this StringBuilder builder, Style? style, int? value) + { + return AppendWithStyle(builder, style, value?.ToString(CultureInfo.InvariantCulture)); + } + + public static StringBuilder AppendWithStyle(this StringBuilder builder, Style? style, string? value) + { + value ??= string.Empty; + + if (style != null) + { + return builder.Append('[') + .Append(style.ToMarkup()) + .Append(']') + .Append(value.EscapeMarkup()) + .Append("[/]"); + } + + return builder.Append(value); + } + } +} diff --git a/src/Spectre.Console/Internal/ExceptionFormatter.cs b/src/Spectre.Console/Internal/ExceptionFormatter.cs index d55168e..afb2612 100644 --- a/src/Spectre.Console/Internal/ExceptionFormatter.cs +++ b/src/Spectre.Console/Internal/ExceptionFormatter.cs @@ -41,12 +41,15 @@ namespace Spectre.Console.Internal { var shortenTypes = (settings.Format & ExceptionFormats.ShortenTypes) != 0; var type = Emphasize(ex.Type, new[] { '.' }, settings.Style.Exception, shortenTypes, settings); + var message = $"[{settings.Style.Message.ToMarkup()}]{ex.Message.EscapeMarkup()}[/]"; return new Markup(string.Concat(type, ": ", message)); } private static Grid GetStackFrames(ExceptionInfo ex, ExceptionSettings settings) { + var styles = settings.Style; + var grid = new Grid(); grid.AddColumn(new GridColumn().PadLeft(2).PadRight(0).NoWrap()); grid.AddColumn(new GridColumn().PadLeft(1).PadRight(0)); @@ -66,14 +69,16 @@ namespace Spectre.Console.Internal // Method var shortenMethods = (settings.Format & ExceptionFormats.ShortenMethods) != 0; - builder.Append(Emphasize(frame.Method, new[] { '.' }, settings.Style.Method, shortenMethods, settings)); - builder.Append('[').Append(settings.Style.Parenthesis.ToMarkup()).Append(']').Append('(').Append("[/]"); + builder.Append(Emphasize(frame.Method, new[] { '.' }, styles.Method, shortenMethods, settings)); + builder.AppendWithStyle(styles.Parenthesis, "("); AppendParameters(builder, frame, settings); - builder.Append('[').Append(settings.Style.Parenthesis.ToMarkup()).Append(']').Append(')').Append("[/]"); + builder.AppendWithStyle(styles.Parenthesis, ")"); if (frame.Path != null) { - builder.Append(" [").Append(settings.Style.Dimmed.ToMarkup()).Append("]in[/] "); + builder.Append(' '); + builder.AppendWithStyle(styles.Dimmed, "in"); + builder.Append(' '); // Path AppendPath(builder, frame, settings); @@ -81,13 +86,13 @@ namespace Spectre.Console.Internal // Line number if (frame.LineNumber != null) { - builder.Append('[').Append(settings.Style.Dimmed.ToMarkup()).Append("]:[/]"); - builder.Append('[').Append(settings.Style.LineNumber.ToMarkup()).Append(']').Append(frame.LineNumber).Append("[/]"); + builder.AppendWithStyle(styles.Dimmed, ":"); + builder.AppendWithStyle(styles.LineNumber, frame.LineNumber); } } grid.AddRow( - $"[{settings.Style.Dimmed.ToMarkup()}]at[/]", + $"[{styles.Dimmed.ToMarkup()}]at[/]", builder.ToString()); } @@ -98,7 +103,7 @@ namespace Spectre.Console.Internal { var typeColor = settings.Style.ParameterType.ToMarkup(); var nameColor = settings.Style.ParameterName.ToMarkup(); - var parameters = frame.Parameters.Select(x => $"[{typeColor}]{x.Type.EscapeMarkup()}[/] [{nameColor}]{x.Name}[/]"); + var parameters = frame.Parameters.Select(x => $"[{typeColor}]{x.Type.EscapeMarkup()}[/] [{nameColor}]{x.Name.EscapeMarkup()}[/]"); builder.Append(string.Join(", ", parameters)); } @@ -146,16 +151,18 @@ namespace Spectre.Console.Internal { if (!compact) { - builder.Append('[').Append(settings.Style.NonEmphasized.ToMarkup()).Append(']') - .Append(type, 0, index + 1).Append("[/]"); + builder.AppendWithStyle( + settings.Style.NonEmphasized, + type.Substring(0, index + 1).EscapeMarkup()); } - builder.Append('[').Append(color.ToMarkup()).Append(']') - .Append(type, index + 1, type.Length - index - 1).Append("[/]"); + builder.AppendWithStyle( + color, + type.Substring(index + 1, type.Length - index - 1).EscapeMarkup()); } else { - builder.Append(type); + builder.Append(type.EscapeMarkup()); } return builder.ToString();