Refactor pretty stack traces

CliFx/CliApplication.cs

@@ -22,7 +22,6 @@ namespace CliFx
         private readonly ITypeActivator _typeActivator;
 
         private readonly HelpTextWriter _helpTextWriter;
-        private readonly ErrorTextWriter _errorTextWriter;
 
         /// <summary>
         /// Initializes an instance of <see cref="CliApplication"/>.
@@ -37,12 +36,8 @@ namespace CliFx
             _typeActivator = typeActivator;
 
             _helpTextWriter = new HelpTextWriter(metadata, console);
-            _errorTextWriter = new ErrorTextWriter(console);
         }
 
-        private void WriteError(string message) => _console.WithForegroundColor(ConsoleColor.Red, () =>
-            _console.Error.WriteLine(message));
-
         private async ValueTask LaunchAndWaitForDebuggerAsync()
         {
             var processId = ProcessEx.GetCurrentProcessId();
@@ -53,7 +48,9 @@ namespace CliFx
             Debugger.Launch();
 
             while (!Debugger.IsAttached)
+            {
                 await Task.Delay(100);
+            }
         }
 
         private void WriteCommandLineInput(CommandInput input)
@@ -105,9 +102,9 @@ namespace CliFx
         }
 
         private ICommand GetCommandInstance(CommandSchema command) =>
-            command != StubDefaultCommand.Schema
+            command != FallbackDefaultCommand.Schema
                 ? (ICommand) _typeActivator.CreateInstance(command.Type)
-                : new StubDefaultCommand();
+                : new FallbackDefaultCommand();
 
         /// <summary>
         /// Runs the application with specified command line arguments and environment variables, and returns the exit code.
@@ -143,7 +140,7 @@ namespace CliFx
                 var command =
                     root.TryFindCommand(input.CommandName) ??
                     root.TryFindDefaultCommand() ??
-                    StubDefaultCommand.Schema;
+                    FallbackDefaultCommand.Schema;
 
                 // Version option
                 if (command.IsVersionOptionAvailable && input.IsVersionOptionSpecified)
@@ -161,7 +158,7 @@ namespace CliFx
 
                 // Help option
                 if (command.IsHelpOptionAvailable && input.IsHelpOptionSpecified ||
-                    command == StubDefaultCommand.Schema && !input.Parameters.Any() && !input.Options.Any())
+                    command == FallbackDefaultCommand.Schema && !input.Parameters.Any() && !input.Options.Any())
                 {
                     _helpTextWriter.Write(root, command, defaultValues);
                     return ExitCode.Success;
@@ -175,7 +172,10 @@ namespace CliFx
                 // This may throw exceptions which are useful only to the end-user
                 catch (CliFxException ex)
                 {
-                    WriteError(ex.ToString());
+                    _console.WithForegroundColor(ConsoleColor.Red, () =>
+                        _console.Error.WriteLine(ex.ToString())
+                    );
+
                     _helpTextWriter.Write(root, command, defaultValues);
 
                     return ExitCode.FromException(ex);
@@ -190,10 +190,14 @@ namespace CliFx
                 // Swallow command exceptions and route them to the console
                 catch (CommandException ex)
                 {
-                    WriteError(ex.ToString());
+                    _console.WithForegroundColor(ConsoleColor.Red, () =>
+                        _console.Error.WriteLine(ex.ToString())
+                    );
 
                     if (ex.ShowHelp)
+                    {
                         _helpTextWriter.Write(root, command, defaultValues);
+                    }
 
                     return ex.ExitCode;
                 }
@@ -204,7 +208,13 @@ namespace CliFx
             // because we still want the IDE to show them to the developer.
             catch (Exception ex) when (!Debugger.IsAttached)
             {
-                _errorTextWriter.WriteError(ex);
+                _console.WithColors(ConsoleColor.White, ConsoleColor.DarkRed, () =>
+                    _console.Error.Write("ERROR:")
+                );
+
+                _console.Error.Write(" ");
+                _console.WriteException(ex);
+
                 return ExitCode.FromException(ex);
             }
         }
@@ -259,10 +269,11 @@ namespace CliFx
                     : 1;
         }
 
+        // Fallback default command used when none is defined in the application
         [Command]
-        private class StubDefaultCommand : ICommand
+        private class FallbackDefaultCommand : ICommand
         {
-            public static CommandSchema Schema { get; } = CommandSchema.TryResolve(typeof(StubDefaultCommand))!;
+            public static CommandSchema Schema { get; } = CommandSchema.TryResolve(typeof(FallbackDefaultCommand))!;
 
             public ValueTask ExecuteAsync(IConsole console) => default;
         }

CliFx/Domain/ErrorTextWriter.cs

@@ -1,226 +0,0 @@
-using System;
-using System.Collections.Generic;
-using System.Text.RegularExpressions;
-
-namespace CliFx.Domain
-{
-    internal class ErrorTextWriter
-    {
-        private const int indent = 4;
-
-        private static readonly ConsoleColor NameColor = ConsoleColor.DarkGray;
-        private static readonly ConsoleColor SpecificNameColor = ConsoleColor.White;
-        private static readonly ConsoleColor MessageColor = ConsoleColor.Red;
-        private static readonly ConsoleColor MethodColor = ConsoleColor.Yellow;
-        private static readonly ConsoleColor ParameterTypeColor = ConsoleColor.Blue;
-        private static readonly ConsoleColor FileColor = ConsoleColor.Yellow;
-        private static readonly ConsoleColor LineColor = ConsoleColor.Blue;
-
-        private static readonly Regex MethodMatcher = new Regex(@"(?<prefix>\S+) (?<name>.*?)(?<methodName>[^\.]+)\(");
-        private static readonly Regex ParameterMatcher = new Regex(@"(?<type>.+? )(?<name>.+?)(?:(?<separator>, )|\))");
-        private static readonly Regex FileMatcher = new Regex(@"(?<prefix>\S+?) (?<path>.*?)(?<file>[^\\/]+?(?:\.\w*)?):[^:]+? (?<line>\d+).*");
-
-        private readonly IConsole _console;
-
-        public ErrorTextWriter(IConsole console)
-        {
-            _console = console;
-        }
-
-        public void WriteError(Exception ex) => WriteError(ex, 0);
-        private void WriteError(Exception ex, int indentLevel)
-        {
-            var indentation = new string(' ', indent * indentLevel);
-            var extraIndentation = new string(' ', indent / 2);
-
-            var exType = ex.GetType();
-
-            // (Fully qualified) type of the exception
-            Write(NameColor, indentation + exType.Namespace + ".");
-            Write(SpecificNameColor, exType.Name);
-            _console.Error.Write(": ");
-
-            // Message
-            Write(MessageColor, ex.Message);
-            _console.Error.WriteLine();
-
-            // Prints the inner exception
-            // with one higher indentation level
-            if (ex.InnerException is Exception innerException)
-            {
-                WriteError(innerException, indentLevel + 1);
-            }
-
-            // Print with formatting when successfully parsing all entries in the stack trace
-            if (ParseStackTrace(ex.StackTrace) is IEnumerable<StackTraceEntry> parsedStackTrace)
-            {
-                // Each step in the stack trace is formated and printed
-                foreach (var entry in parsedStackTrace)
-                {
-                    _console.Error.Write(indentation + extraIndentation);
-
-                    WriteMethodDescriptor(entry.MethodPrefix, entry.MethodName, entry.MethodSpecificName);
-
-                    WriteParameters(entry.Parameters);
-
-                    _console.Error.Write(entry.FilePrefix);
-                    _console.Error.Write("\n" + indentation + extraIndentation + extraIndentation);
-                    WriteFileDescriptor(entry.FilePath, entry.FileName, entry.FileLine);
-
-                    _console.Error.WriteLine();
-                }
-            }
-            else
-            {
-                // Parsing failed. Print without formatting.
-                foreach (var trace in ex.StackTrace.Split('\n'))
-                {
-                    _console.Error.WriteLine(indentation + trace);
-                }
-            }
-        }
-
-        private void WriteMethodDescriptor(string prefix, string name, string methodName)
-        {
-            _console.Error.Write(prefix + " ");
-            Write(NameColor, name);
-            Write(MethodColor, methodName);
-        }
-
-        private void WriteParameters(IEnumerable<ParameterEntry> parameters)
-        {
-            _console.Error.Write("(");
-            foreach (var parameter in parameters)
-            {
-                Write(ParameterTypeColor, parameter.Type);
-                Write(SpecificNameColor, parameter.Name);
-
-                if (parameter.Separator is string separator)
-                {
-                    _console.Error.Write(separator);
-                }
-            }
-            _console.Error.Write(") ");
-        }
-
-        private void WriteFileDescriptor(string path, string fileName, string lineNumber)
-        {
-            Write(NameColor, path);
-
-            Write(FileColor, fileName);
-            _console.Error.Write(":");
-            Write(LineColor, lineNumber);
-        }
-
-        private void Write(ConsoleColor color, string value)
-        {
-            _console.WithForegroundColor(color, () => _console.Error.Write(value));
-        }
-
-        private IEnumerable<StackTraceEntry>? ParseStackTrace(string stackTrace)
-        {
-            IList<StackTraceEntry> stackTraceEntries = new List<StackTraceEntry>();
-            foreach (var trace in stackTrace.Split('\n'))
-            {
-                var methodMatch = MethodMatcher.Match(trace);
-                var parameterMatches = ParameterMatcher.Matches(trace, methodMatch.Index + methodMatch.Length);
-                var fileMatch = FileMatcher.Match(
-                    trace,
-                    parameterMatches.Count switch
-                    {
-                        0 => methodMatch.Index + methodMatch.Length + 1,
-                        int c => parameterMatches[c - 1].Index + parameterMatches[c - 1].Length
-                    }
-                );
-
-                if (fileMatch.Index + fileMatch.Length != trace.Length)
-                {
-                    // Didnt match the whole trace
-                    return null;
-                }
-
-                try
-                {
-                    IList<ParameterEntry> parameters = new List<ParameterEntry>();
-                    foreach (Match match in parameterMatches)
-                    {
-                        parameters.Add(new ParameterEntry(
-                            match.Groups["type"].Success ? match.Groups["type"].Value : throw new Exception("type"),
-                            match.Groups["name"].Success ? match.Groups["name"].Value : throw new Exception("name"),
-                            match.Groups["separator"]?.Value // If this is null, it's just the last parameter
-                        ));
-                    }
-
-                    stackTraceEntries.Add(new StackTraceEntry(
-                        methodMatch.Groups["prefix"].Success ? methodMatch.Groups["prefix"].Value : throw new Exception("prefix"),
-                        methodMatch.Groups["name"].Success ? methodMatch.Groups["name"].Value : throw new Exception("name"),
-                        methodMatch.Groups["methodName"].Success ? methodMatch.Groups["methodName"].Value : throw new Exception("methodName"),
-                        parameters,
-                        fileMatch.Groups["prefix"].Success ? fileMatch.Groups["prefix"].Value : throw new Exception("prefix"),
-                        fileMatch.Groups["path"].Success ? fileMatch.Groups["path"].Value : throw new Exception("path"),
-                        fileMatch.Groups["file"].Success ? fileMatch.Groups["file"].Value : throw new Exception("file"),
-                        fileMatch.Groups["line"].Success ? fileMatch.Groups["line"].Value : throw new Exception("line")
-                    ));
-                }
-                catch
-                {
-                    // One of the required groups failed to match
-                    return null;
-                }
-            }
-
-            return stackTraceEntries;
-        }
-
-        private readonly struct StackTraceEntry
-        {
-            public string MethodPrefix { get; }
-            public string MethodName { get; }
-            public string MethodSpecificName { get; }
-
-            public IEnumerable<ParameterEntry> Parameters { get; }
-
-            public string FilePrefix { get; }
-            public string FilePath { get; }
-            public string FileName { get; }
-            public string FileLine { get; }
-
-            public StackTraceEntry(
-                string methodPrefix,
-                string methodName,
-                string methodSpecificName,
-                IEnumerable<ParameterEntry> parameters,
-                string filePrefix,
-                string filePath,
-                string fileName,
-                string fileLine)
-            {
-                MethodPrefix = methodPrefix;
-                MethodName = methodName;
-                MethodSpecificName = methodSpecificName;
-                Parameters = parameters;
-                FilePrefix = filePrefix;
-                FilePath = filePath;
-                FileName = fileName;
-                FileLine = fileLine;
-            }
-        }
-
-        private readonly struct ParameterEntry
-        {
-            public string Type { get; }
-            public string Name { get; }
-            public string? Separator { get; }
-
-            public ParameterEntry(
-                string type,
-                string name,
-                string? separator)
-            {
-                Type = type;
-                Name = name;
-                Separator = separator;
-            }
-        }
-    }
-}

CliFx/IConsole.cs

@@ -1,6 +1,7 @@
 using System;
 using System.IO;
 using System.Threading;
+using CliFx.Internal;
 
 namespace CliFx
 {
@@ -85,7 +86,10 @@ namespace CliFx
         /// <summary>
         /// Sets console foreground color, executes specified action, and sets the color back to the original value.
         /// </summary>
-        public static void WithForegroundColor(this IConsole console, ConsoleColor foregroundColor, Action action)
+        public static void WithForegroundColor(
+            this IConsole console,
+            ConsoleColor foregroundColor,
+            Action action)
         {
             var lastColor = console.ForegroundColor;
             console.ForegroundColor = foregroundColor;
@@ -98,7 +102,10 @@ namespace CliFx
         /// <summary>
         /// Sets console background color, executes specified action, and sets the color back to the original value.
         /// </summary>
-        public static void WithBackgroundColor(this IConsole console, ConsoleColor backgroundColor, Action action)
+        public static void WithBackgroundColor(
+            this IConsole console,
+            ConsoleColor backgroundColor,
+            Action action)
         {
             var lastColor = console.BackgroundColor;
             console.BackgroundColor = backgroundColor;
@@ -111,7 +118,120 @@ namespace CliFx
         /// <summary>
         /// Sets console foreground and background colors, executes specified action, and sets the colors back to the original values.
         /// </summary>
-        public static void WithColors(this IConsole console, ConsoleColor foregroundColor, ConsoleColor backgroundColor, Action action) =>
+        public static void WithColors(
+            this IConsole console,
+            ConsoleColor foregroundColor,
+            ConsoleColor backgroundColor,
+            Action action) =>
             console.WithForegroundColor(foregroundColor, () => console.WithBackgroundColor(backgroundColor, action));
+
+        private static void WriteException(
+            this IConsole console,
+            Exception exception,
+            int indentLevel)
+        {
+            var exceptionType = exception.GetType();
+
+            var indentationShared = new string(' ', 4 * indentLevel);
+            var indentationLocal = new string(' ', 2);
+
+            // Fully qualified exception type
+            console.Error.Write(indentationShared);
+            console.WithForegroundColor(ConsoleColor.DarkGray, () =>
+                console.Error.Write(exceptionType.Namespace + ".")
+            );
+            console.WithForegroundColor(ConsoleColor.White, () =>
+                console.Error.Write(exceptionType.Name)
+            );
+            console.Error.Write(": ");
+
+            // Exception message
+            console.WithForegroundColor(ConsoleColor.Red, () => console.Error.WriteLine(exception.Message));
+
+            // Recurse into inner exceptions
+            if (exception.InnerException != null)
+            {
+                console.WriteException(exception.InnerException, indentLevel + 1);
+            }
+
+            // Try to parse and pretty-print the stack trace
+            try
+            {
+                foreach (var stackFrame in StackFrame.ParseMany(exception.StackTrace))
+                {
+                    console.Error.Write(indentationShared + indentationLocal);
+
+                    // "at"
+                    console.Error.Write(stackFrame.Prefix + " ");
+
+                    // "CliFx.Demo.Commands.BookAddCommand."
+                    console.WithForegroundColor(ConsoleColor.DarkGray, () =>
+                        console.Error.Write(stackFrame.ParentType)
+                    );
+
+                    // "ExecuteAsync"
+                    console.WithForegroundColor(ConsoleColor.Yellow, () =>
+                        console.Error.Write(stackFrame.MethodName)
+                    );
+
+                    console.Error.Write("(");
+
+                    foreach (var parameter in stackFrame.Parameters)
+                    {
+                        // "IConsole"
+                        console.WithForegroundColor(ConsoleColor.Blue, () =>
+                            console.Error.Write(parameter.Type)
+                        );
+
+                        // "console"
+                        console.WithForegroundColor(ConsoleColor.White, () =>
+                            console.Error.Write(parameter.Name)
+                        );
+
+                        // ", '
+                        if (parameter.Separator != null)
+                        {
+                            console.Error.Write(parameter.Separator);
+                        }
+                    }
+
+                    console.Error.Write(") ");
+
+                    // "in"
+                    console.Error.Write(stackFrame.LocationPrefix);
+                    console.Error.Write("\n" + indentationShared + indentationLocal + indentationLocal);
+
+                    // "E:\Projects\Softdev\CliFx\CliFx.Demo\Commands\"
+                    console.WithForegroundColor(ConsoleColor.DarkGray, () =>
+                        console.Error.Write(stackFrame.DirectoryPath)
+                    );
+
+                    // "BookAddCommand.cs"
+                    console.WithForegroundColor(ConsoleColor.Yellow, () =>
+                        console.Error.Write(stackFrame.FileName)
+                    );
+
+                    console.Error.Write(":");
+
+                    // "35"
+                    console.WithForegroundColor(ConsoleColor.Blue, () =>
+                        console.Error.Write(stackFrame.LineNumber)
+                    );
+
+                    console.Error.WriteLine();
+                }
+            }
+            // If any point of parsing has failed - print the stack trace without any formatting
+            catch
+            {
+                console.Error.WriteLine(exception.StackTrace);
+            }
+        }
+
+        //Should this be public?
+        internal static void WriteException(
+            this IConsole console,
+            Exception exception) =>
+            console.WriteException(exception, 0);
     }
 }

CliFx/Internal/Extensions/StringExtensions.cs

@@ -6,6 +6,11 @@ namespace CliFx.Internal.Extensions
 {
     internal static class StringExtensions
     {
+        public static string? NullIfWhiteSpace(this string str) =>
+            !string.IsNullOrWhiteSpace(str)
+                ? str
+                : null;
+
         public static string Repeat(this char c, int count) => new string(c, count);
 
         public static string AsString(this char c) => c.Repeat(1);

CliFx/Internal/Polyfills.cs

@@ -17,6 +17,9 @@ namespace System
 
         public static bool EndsWith(this string str, char c) =>
             str.Length > 0 && str[str.Length - 1] == c;
+
+        public static string[] Split(this string str, char separator, StringSplitOptions splitOptions) =>
+            str.Split(new[] {separator}, splitOptions);
     }
 }
 

CliFx/Internal/StackFrame.cs

@@ -0,0 +1,130 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text.RegularExpressions;
+using CliFx.Internal.Extensions;
+
+namespace CliFx.Internal
+{
+    internal class StackFrameParameter
+    {
+        public string Type { get; }
+
+        public string Name { get; }
+
+        public string? Separator { get; }
+
+        public StackFrameParameter(
+            string type,
+            string name,
+            string? separator)
+        {
+            Type = type;
+            Name = name;
+            Separator = separator;
+        }
+    }
+
+    internal partial class StackFrame
+    {
+        public string Prefix { get; }
+
+        public string ParentType { get; }
+
+        public string MethodName { get; }
+
+        public IReadOnlyList<StackFrameParameter> Parameters { get; }
+
+        public string LocationPrefix { get; }
+
+        public string DirectoryPath { get; }
+
+        public string FileName { get; }
+
+        public string LineNumber { get; }
+
+        public StackFrame(
+            string prefix,
+            string parentType,
+            string methodName,
+            IReadOnlyList<StackFrameParameter> parameters,
+            string locationPrefix,
+            string directoryPath,
+            string fileName,
+            string lineNumber)
+        {
+            Prefix = prefix;
+            ParentType = parentType;
+            MethodName = methodName;
+            Parameters = parameters;
+            LocationPrefix = locationPrefix;
+            DirectoryPath = directoryPath;
+            FileName = fileName;
+            LineNumber = lineNumber;
+        }
+    }
+
+    internal partial class StackFrame
+    {
+        private static readonly Regex MethodMatcher =
+            new Regex(@"(?<prefix>\S+) (?<name>.*?)(?<methodName>[^\.]+)\(");
+
+        private static readonly Regex ParameterMatcher =
+            new Regex(@"(?<type>.+? )(?<name>.+?)(?:(?<separator>, )|\))");
+
+        private static readonly Regex FileMatcher =
+            new Regex(@"(?<prefix>\S+?) (?<path>.*?)(?<file>[^\\/]+?(?:\.\w*)?):[^:]+? (?<line>\d+).*");
+
+        public static StackFrame Parse(string stackFrame)
+        {
+            var methodMatch = MethodMatcher.Match(stackFrame);
+
+            var parameterMatches = ParameterMatcher.Matches(stackFrame, methodMatch.Index + methodMatch.Length)
+                .Cast<Match>()
+                .ToArray();
+
+            var fileMatch = FileMatcher.Match(
+                stackFrame,
+                parameterMatches.Length switch
+                {
+                    0 => methodMatch.Index + methodMatch.Length + 1,
+                    _ => parameterMatches[parameterMatches.Length - 1].Index +
+                         parameterMatches[parameterMatches.Length - 1].Length
+                }
+            );
+
+            // Ensure everything was parsed successfully
+            var isSuccessful =
+                methodMatch.Success &&
+                parameterMatches.All(m => m.Success) &&
+                fileMatch.Success &&
+                fileMatch.Index + fileMatch.Length == stackFrame.Length;
+
+            if (!isSuccessful)
+            {
+                throw new FormatException("Failed to parse stack frame.");
+            }
+
+            var parameters = parameterMatches
+                .Select(match => new StackFrameParameter(
+                    match.Groups["type"].Value,
+                    match.Groups["name"].Value,
+                    match.Groups["separator"].Value.NullIfWhiteSpace()
+                )).ToArray();
+
+            return new StackFrame(
+                methodMatch.Groups["prefix"].Value,
+                methodMatch.Groups["name"].Value,
+                methodMatch.Groups["methodName"].Value,
+                parameters,
+                fileMatch.Groups["prefix"].Value,
+                fileMatch.Groups["path"].Value,
+                fileMatch.Groups["file"].Value,
+                fileMatch.Groups["line"].Value
+            );
+        }
+
+        public static IReadOnlyList<StackFrame> ParseMany(string stackTrace) =>
+            stackTrace.Split('\n', StringSplitOptions.RemoveEmptyEntries).Select(Parse).ToArray();
+    }
+}