6
\$\begingroup\$

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);

ConsoleColorizer

\$\endgroup\$
  • 1
    \$\begingroup\$ Gets memories of QBASIC looking at that white-on-blue and red-on-gray text \$\endgroup\$ – Mathieu Guindon Oct 24 '16 at 20:55
  • \$\begingroup\$ @Mat'sMug the colors are purely coincidental but now I see it too ;-] \$\endgroup\$ – t3chb0t Oct 25 '16 at 6:46
  • \$\begingroup\$ You might be interested in this related question & project. codereview.stackexchange.com/q/70270/41243 \$\endgroup\$ – RubberDuck Oct 25 '16 at 16:41
  • 1
    \$\begingroup\$ @RubberDuck that project is exactly how I didn't want it to be - and I had before ;-) I prefer to use a html/xml like syntax for a template to render a line with multiple styles rather then several lines of Something.Write(...) I downloaded the source and saw it ;-] however the Idea with predefined styles is good so I'll probably add it to my colorizer too. \$\endgroup\$ – t3chb0t Oct 25 '16 at 17:06
3
\$\begingroup\$

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;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ I like your suggestions. Together with @RobH's review they make it much more reliable and prittier ;-) \$\endgroup\$ – t3chb0t Oct 25 '16 at 9:04
  • 1
    \$\begingroup\$ There is a reason why I put the Enum.TryParse inside an if. The TryParse will reset the out variable, see result = default(TEnum); at Reference Source so the default value won't be the current ConsoleColor.ForegroundColor but Black (0) if an attribute wasn't there - but I can fix it ;-) \$\endgroup\$ – t3chb0t Oct 25 '16 at 9:22
  • \$\begingroup\$ I missed that piece. Good catch \$\endgroup\$ – Heslacher Oct 25 '16 at 9:26
3
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.