Console colorizer

Question

To be able to apply various colors to the console I created a ConosoleColorizer. It's really simple. It just takes an XML and renders it to the console with the colors specified.

The element names are actually optional and can be any names. They are only required to parse the XML. What matters are the attribute and color names. I didn't know how to solve it with less effort without inventing a new markup.

internal class ConsoleColorizer
{
    public static void Render(string xml)
    {
        Render(XElement.Parse(xml).Nodes());
    }

    public static void Render(IEnumerable<XNode> xNodes)
    {
        Render(xNodes, null, null);
    }

    private static void Render(IEnumerable<XNode> xNodes, ConsoleColor? lastForegroundColor, ConsoleColor? lastBackgroundColor)
    {
        foreach (var xChildNode in xNodes)
        {
            var xElement = xChildNode as XElement;
            if (xElement != null)
            {
                Render(
                    xElement.Nodes(),
                    SetForegroundColor(xElement),
                    SetBackgroundColor(xElement)
                );
            }
            else
            {
                RestoreForegroundColor(lastForegroundColor);
                RestoreBackgroundColor(lastBackgroundColor);
                Console.Write(((XText)xChildNode).Value);
            }
        }
        Console.ResetColor();
    }

    private static ConsoleColor? SetForegroundColor(XElement xElement)
    {
        var foregroundColor = (ConsoleColor)0;
        if (Enum.TryParse<ConsoleColor>(xElement.Attribute("fg")?.Value, true, out foregroundColor))
        {
            return Console.ForegroundColor = foregroundColor;
        }
        return null;
    }

    private static ConsoleColor? SetBackgroundColor(XElement xElement)
    {
        var backgroundColor = (ConsoleColor)0;
        if (Enum.TryParse<ConsoleColor>(xElement.Attribute("bg")?.Value, true, out backgroundColor))
        {
            return Console.BackgroundColor = backgroundColor;
        }
        return null;
    }

    private static void RestoreForegroundColor(ConsoleColor? consoleColor)
    {
        if (consoleColor.HasValue)
        {
            Console.ForegroundColor = consoleColor.Value;
        }
    }

    private static void RestoreBackgroundColor(ConsoleColor? consoleColor)
    {
        if (consoleColor.HasValue)
        {
            Console.BackgroundColor = consoleColor.Value;
        }
    }
}

Example:

var xml = @"<line>Hallo <color fg=""yellow"">colored</color> console! <color fg=""darkred"" bg=""darkgray"">These are <color fg=""white"" bg=""blue"">nested</color> colors</color>.</line>";
ConsoleColorizer.Render(xml);

Answer 1

The code in question looks mostly good to me, but it could be enhanced a little bit.

private static void Render()

The call to Console.ResetColor(); doesn't belong here because it isn't necessary to reset the colors each time.

Remarks from link above:

The foreground and background colors are restored to the colors that existed when the current process began.

So it is sufficient to call that method at the end of the Render(IEnumerable<XNode> xNodes) method.

The passed in lastForegroundColor and lastBackgroundColor are only needed if one of the childnodes is a XElement so if we extract the rendering of a XElement to a separate method, the former Render() method would look after renaming to RenderInternal() like so

private static void RenderInternal(IEnumerable<XNode> xNodes)
{
    foreach (var xChildNode in xNodes)
    {
        var xElement = xChildNode as XElement;
        if (xElement != null)
        {
            RenderInternal(xElement);
        }
        else
        {
            Console.Write(((XText)xChildNode).Value);
        }
    }
}  

The extracted RenderInternal(XElement) method could look like so, if we just use the current implemented methods

private static void RenderInternal(XElement xElement)
{
    ConsoleColor lastForegroundColor = Console.ForegroundColor;
    ConsoleColor lastBackgroundColor = Console.BackgroundColor;

    SetForegroundColor(xElement);
    SetBackgroundColor(xElement);

    RenderInternal(xElement.Nodes());

    RestoreForegroundColor(lastForegroundColor);
    RestoreBackgroundColor(lastBackgroundColor);

}  

which removes the strangeness of a SetXxx method to return something. But hey, I just don't like how this is looking. So let us introduce a struct ConsoleColors to hold both the Foreground- and the Backgroundcolor like so

public struct ConsoleColors
{
    public ConsoleColor BackgroundColor { get;}
    public ConsoleColor ForegroundColor { get;}
    public static ConsoleColors Current
    {
        get
        {
            return new ConsoleColors(Console.ForegroundColor, Console.BackgroundColor);
        }
    }
    
    public ConsoleColors(ConsoleColor foregroundColor, ConsoleColor backgroundColor)
        :this()
    {
        ForegroundColor = foregroundColor;
        BackgroundColor = backgroundColor;
    }
}  

and add a method which sets a ConsoleColors struct like so

private static void SetColors(ConsoleColors colors)
{
    Console.ForegroundColor = colors.ForegroundColor;
    Console.BackgroundColor = colors.BackgroundColor;
}  

and refactor the RenderInternal(XElement) method like so

private static void RenderInternal(XElement xElement)
{
    ConsoleColors savedColors = ConsoleColors.Current;

    SetForegroundColor(xElement);
    SetBackgroundColor(xElement);

    RenderInternal(xElement.Nodes());

    SetColors(savedColors);
}

which looks better, but still has the calls SetForegroundColor and SetBackgroundColor. So we can add an extension method which turns a XElement to a ConsoleColors so we can use the SetColors method.

public static ConsoleColors ToConsoleColors(this XElement xElement, string foregroundAttributeName = "fg", string backgroundAttributeName = "bg")
{
    if (xElement == null) { return ConsoleColors.Current; }

    var foregroundColor = Console.ForegroundColor;
    Enum.TryParse<ConsoleColor>(xElement.Attribute(foregroundAttributeName)?.Value, true, out foregroundColor);

    var backgroundColor = Console.BackgroundColor;
    Enum.TryParse<ConsoleColor>(xElement.Attribute(backgroundAttributeName)?.Value, true, out backgroundColor);

    return new ConsoleColors(foregroundColor, backgroundColor);
}  

which leads to

    internal class ConsoleColorizer
    {
        public static void Render(string xml)
        {
            Render(XElement.Parse(xml).Nodes());
        }

        public static void Render(IEnumerable<XNode> xNodes)
        {
            RenderInternal(xNodes);
            Console.ResetColor();
        }

        private static void RenderInternal(IEnumerable<XNode> xNodes)
        {
            foreach (var xChildNode in xNodes)
            {
                var xElement = xChildNode as XElement;
                if (xElement != null)
                {
                    RenderInternal(xElement);
                }
                else
                {
                    Console.Write(((XText)xChildNode).Value);
                }
            }
        }
        private static void RenderInternal(XElement xElement)
        {
            ConsoleColors savedColors = ConsoleColors.Current;

            SetColors(xElement.ToConsoleColors());

            RenderInternal(xElement.Nodes());

            SetColors(savedColors);
        }

        private static void SetColors(ConsoleColors colors)
        {
            Console.ForegroundColor = colors.ForegroundColor;
            Console.BackgroundColor = colors.BackgroundColor;
        }
    }
}

Answer 2

This is dangerous: ((XText)xChildNode) you can't make the guarantee that the child node will be a text node. For example, say that I have this console message:

var message = 
@"<text>
    hallo
    <!-- tag name is irrelevant, it's all in the attribute -->
    <color fg=""yellow"">colored</color> 
    console!
</text>";
ConsoleColorizer.Render(message);

That's an InvalidCastException right there so only part of the message will be printed out.

---

var backgroundColor = (ConsoleColor)0; is odd, it's about the only place I don't use var... ConsoleColor backgroundColor; is much clearer.