From 7ce4e69cf89434a0d1537efd6dee022cdcf6447b Mon Sep 17 00:00:00 2001
From: TEC <git@tecosaur.net>
Date: Sun, 11 Feb 2024 17:36:09 +0800
Subject: [PATCH] More consistent legacy color conversion

When a given "legacy color" argument is not in fact a recognised color
name, `legacy_color` should return nil, and indeed it does when the
argument is a string.

However, the color symbol method will just return the symbol argument if
it is not recognised, creating an inconsistency in the behaviour.
Instead, it should return nothing, as the string case does.

This change is made, and a little of the surrounding code rearranged for
clarity.
---
 docs/src/internals.md |  1 +
 src/legacy.jl         | 43 +++++++++++++++++++++++++++----------------
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/docs/src/internals.md b/docs/src/internals.md
index e997ba1..97d9b48 100644
--- a/docs/src/internals.md
+++ b/docs/src/internals.md
@@ -10,6 +10,7 @@ StyledStrings.ANSI_4BIT_COLORS
 StyledStrings.FACES
 StyledStrings.HTML_BASIC_COLORS
 StyledStrings.Legacy.ANSI_256_COLORS
+StyledStrings.Legacy.NAMED_COLORS
 StyledStrings.Legacy.RENAMED_COLORS
 StyledStrings.Legacy.legacy_color
 StyledStrings.Legacy.load_env_colors!
diff --git a/src/legacy.jl b/src/legacy.jl
index e7c5669..50ac7c7 100644
--- a/src/legacy.jl
+++ b/src/legacy.jl
@@ -8,6 +8,15 @@ module Legacy
 
 using ..StyledStrings: SimpleColor, Face, loadface!, face!
 
+"""
+    legacy_color(color::Union{String, Symbol, Int})
+
+Attempt to obtain a `SimpleColor` for a "legacy" color value `color`.
+
+When this is not possible, `nothing` is returned.
+"""
+function legacy_color end
+
 """
 A mapping from 256-color codes indicies to 8-bit colours.
 """
@@ -51,6 +60,19 @@ const ANSI_256_COLORS =
          0x8a8a8a, 0x949494, 0x9e9e9e, 0xa8a8a8, 0xb2b2b2, 0xbcbcbc, 0xc6c6c6,
          0xd0d0d0, 0xdadada, 0xe4e4e4, 0xeeeeee])
 
+legacy_color(color256::Int) = get(ANSI_256_COLORS, color256+1, nothing)
+
+"""
+A list of all named colors recognised, including both the old `light_*` and new
+`bright_*` named colors.
+"""
+const NAMED_COLORS =
+    (:black, :red, :green, :yellow, :blue, :magenta, :cyan, :white,
+     :bright_black, :grey, :gray, :bright_red, :bright_green, :bright_yellow,
+     :bright_blue, :bright_magenta, :bright_cyan, :bright_white, :light_black,
+     :light_red, :light_green, :light_yellow, :light_blue, :light_magenta,
+     :light_cyan, :light_white)
+
 """
 A mapping from old named colours to the new names, specifically from `light_*`
 to `bright_*`.
@@ -65,24 +87,13 @@ const RENAMED_COLORS = Dict{Symbol, Symbol}(
     :light_cyan    => :bright_cyan,
     :light_white   => :bright_white)
 
-legacy_color(color::Symbol) = SimpleColor(get(RENAMED_COLORS, color, color))
-legacy_color(color256::Int) = get(ANSI_256_COLORS, color256+1, nothing)
-
-"""
-    legacy_color(color::Union{String, Symbol, Int})
-
-Attempt to obtain a `SimpleColor` for a "legacy" color value `color`.
+legacy_color(color::Symbol) =
+    if color in NAMED_COLORS
+        SimpleColor(get(RENAMED_COLORS, color, color))
+    end
 
-When this is not possible, `nothing` is returned.
-"""
 function legacy_color(color::String)
-    namedcolours = ("black", "red", "green", "yellow", "blue", "magenta",
-                    "cyan", "white", "bright_black", "grey", "gray",
-                    "bright_red", "bright_green", "bright_yellow",
-                    "bright_blue", "bright_magenta", "bright_cyan",
-                    "bright_white", "light_black", "light_red", "light_green",
-                    "light_yellow", "light_blue", "light_magenta", "light_cyan",
-                    "light_white")
+    namedcolours = String.(NAMED_COLORS)
     if color in namedcolours
         legacy_color(Symbol(color))
     elseif 0 <= (color256 = something(tryparse(Int, color), -1)) <= 255