From f6a1a404717c43f505d416085b2c3eaf93cff0e2 Mon Sep 17 00:00:00 2001
From: Alexey Golub <tyrrrz@gmail.com>
Date: Fri, 8 May 2020 16:33:28 +0300
Subject: [PATCH] Cleanup

---
 CliFx/Domain/CommandArgumentSchema.cs | 61 +++++++++------------------
 CliFx/Domain/CommandOptionSchema.cs   |  4 --
 CliFx/Domain/HelpTextWriter.cs        |  4 +-
 3 files changed, 21 insertions(+), 48 deletions(-)

diff --git a/CliFx/Domain/CommandArgumentSchema.cs b/CliFx/Domain/CommandArgumentSchema.cs
index 9b219bf..6bb06ca 100644
--- a/CliFx/Domain/CommandArgumentSchema.cs
+++ b/CliFx/Domain/CommandArgumentSchema.cs
@@ -10,7 +10,6 @@ namespace CliFx.Domain
 {
     internal abstract partial class CommandArgumentSchema
     {
-        private IReadOnlyList<string>? _validValues;
         public PropertyInfo Property { get; }
 
         public string? Description { get; }
@@ -19,52 +18,12 @@ namespace CliFx.Domain
 
         public bool IsScalar => TryGetEnumerableArgumentUnderlyingType() == null;
 
-        public IReadOnlyList<string> GetValidValues() => _validValues ??
-            (_validValues = EnumerateValidValues().ToList().AsReadOnly());
-
         protected CommandArgumentSchema(PropertyInfo property, string? description)
         {
             Property = property;
             Description = description;
         }
 
-        private IEnumerable<string> EnumerateValidValues()
-        {
-            var propertyType = Property?.PropertyType;
-
-            // Property can actually be null here due to damn it operators 
-            // in CommandOptionSchema lines 103 and 106, so we have to check 
-            // for now. In such case that it is null, let's end early.
-            if (propertyType is null)
-            {
-                yield break;
-            }
-
-            // If 'propertyType' is nullable, this will return a non-null value.
-            var underlyingType = propertyType.GetNullableUnderlyingType();
-
-            // If 'propertyType' is nullable, 'underlying' type will be not null.
-            if (underlyingType is object)
-            {
-                // Handle nullable num.
-                if (underlyingType.IsEnum)
-                {
-                    // Reasign so we can do the 'foreach' over the enum values 
-                    // only once at the end of the method.
-                    propertyType = underlyingType;
-                }
-            }
-
-            // Handle non-nullable enums or nullable enums that were "unwrapped".
-            if (propertyType.IsEnum)
-            {
-                foreach (var value in Enum.GetValues(propertyType))
-                {
-                    yield return value.ToString();
-                }
-            }
-        }
-
         private Type? TryGetEnumerableArgumentUnderlyingType() =>
             Property.PropertyType != typeof(string)
                 ? Property.PropertyType.GetEnumerableUnderlyingType()
@@ -156,7 +115,25 @@ namespace CliFx.Domain
             Property.SetValue(command, Convert(values));
 
         public void Inject(ICommand command, params string[] values) =>
-            Inject(command, (IReadOnlyList<string>)values);
+            Inject(command, (IReadOnlyList<string>) values);
+
+        public IReadOnlyList<string> GetValidValues()
+        {
+            var result = new List<string>();
+
+            // Some arguments may have this as null due to a hack that enables built-in options
+            if (Property == null)
+                return result;
+
+            var underlyingPropertyType =
+                Property.PropertyType.GetNullableUnderlyingType() ?? Property.PropertyType;
+
+            // Enum
+            if (underlyingPropertyType.IsEnum)
+                result.AddRange(Enum.GetNames(underlyingPropertyType));
+
+            return result;
+        }
     }
 
     internal partial class CommandArgumentSchema
diff --git a/CliFx/Domain/CommandOptionSchema.cs b/CliFx/Domain/CommandOptionSchema.cs
index b7fb272..cd9ca89 100644
--- a/CliFx/Domain/CommandOptionSchema.cs
+++ b/CliFx/Domain/CommandOptionSchema.cs
@@ -43,10 +43,6 @@ namespace CliFx.Domain
             ShortName != null &&
             ShortName == shortName;
 
-        public bool MatchesNameOrShortName(string? name, char? shortName) =>
-            MatchesName(name) ||
-            MatchesShortName(shortName);
-
         public bool MatchesNameOrShortName(string alias) =>
             MatchesName(alias) ||
             alias.Length == 1 && MatchesShortName(alias.Single());
diff --git a/CliFx/Domain/HelpTextWriter.cs b/CliFx/Domain/HelpTextWriter.cs
index c764ef7..3be9f95 100644
--- a/CliFx/Domain/HelpTextWriter.cs
+++ b/CliFx/Domain/HelpTextWriter.cs
@@ -61,7 +61,7 @@ namespace CliFx.Domain
                 }
                 else
                 {
-                    Render(" ");
+                    RenderIndent(margin);
                 }
             }
 
@@ -264,7 +264,7 @@ namespace CliFx.Domain
                     // Environment variable
                     if (!string.IsNullOrWhiteSpace(option.EnvironmentVariableName))
                     {
-                        Render($"(Environment variable: {option.EnvironmentVariableName})");                        
+                        Render($"Environment variable: {option.EnvironmentVariableName}");
                     }
 
                     RenderNewLine();