Refactor help output formatter to be less crazy

opentap · Jun 19, 2024 · eda4ffe · eda4ffe
1 parent 7868aa3
commit eda4ffe
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 37 deletions.
diff --git a/Package/Image/ImageHelper.cs b/Package/Image/ImageHelper.cs
@@ -30,9 +30,9 @@ static bool IsXml(this string xmlData)
         {
             return xmlData.Trim().Substring(0, 1).IndexOfAny(new[] { '<' }) == 0;
         }
-        static ImageSpecifier ParseCommaSeparated(this string xmlData)
+        static ImageSpecifier ParseCommaSeparated(this string imageString)
         {
-            var pkgStrings = xmlData.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
+            var pkgStrings = imageString.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
 
             var list = new List<PackageSpecifier>();
             foreach (var pkg in pkgStrings)

diff --git a/Package/PackageActions/ImageInstall.cs b/Package/PackageActions/ImageInstall.cs
@@ -14,7 +14,9 @@ internal class ImageInstallAction : IsolatedPackageAction
         /// <summary>
         /// Path to Image file containing XML or JSON formatted Image specification, or just the string itself, e.g "REST-API,TUI:beta".
         /// </summary>
-        [UnnamedCommandLineArgument("image", Required = true, Description = "A string describing the image such as 'OpenTAP:9.24,TUI:beta,CSV', or the path to a file containing an XML or JSON formatted image specification.")]
+        [UnnamedCommandLineArgument("image", Required = true, Description = "The image to install. Supported formats:\n" +
+                "A string describing the image. Example: 'OpenTAP:9.24,TUI:beta,CSV'\n" + 
+                "A path to a file containing an XML or JSON formatted image specification.")]
         public string ImagePath { get; set; }
 
         /// <summary>

diff --git a/Package/PackageActions/LockingPackageAction.cs b/Package/PackageActions/LockingPackageAction.cs
@@ -60,7 +60,7 @@ public abstract class LockingPackageAction : PackageAction
 
         internal static string GetLocalInstallationDir()
         {
-            return Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
+            return Installation.Current.Directory;
         }
 
         internal static string GuessHostOS()

diff --git a/Shared/ArgumentsParser.cs b/Shared/ArgumentsParser.cs
@@ -206,6 +206,7 @@ public bool CompareTo(string arg)
     internal class ArgumentCollection : Dictionary<string, Argument>
     {
         public string[] UnnamedArguments { get; set; }
+        public List<IMemberData> UnnamedArgumentData { get; set; }
 
         public List<string> UnknownsOptions { get; set; }
         public List<Argument> MissingArguments { get; set; }
@@ -288,58 +289,91 @@ public ArgumentCollection CreateDefault()
         }
 
         /// <summary>
-        /// Converts the ArgumentCollection to a help-string.
+        /// This is a simple abstraction over unnamed and named arguments to simplify output formatting.
         /// </summary>
         /// <returns></returns>
-        public override string ToString()
+        class OptionWrapper
         {
-            StringBuilder output = new StringBuilder();
+            public UnnamedCommandLineArgument Positional { get; }
+            public Argument Argument { get; }
 
-            // Compute the options' width
-            int width = 0;
-            List<string> optList = new List<string>();
-            foreach (var option in this)
+            public OptionWrapper(Argument argument)
+            {
+                Argument = argument;
+            }
+
+            public OptionWrapper(UnnamedCommandLineArgument positional)
             {
-                var opt = option.Value;
+                Positional = positional;
+            }
 
-                if (opt.IsVisible == false)
-                    continue;
+            public string Description()
+            {
+                return Positional != null ? Positional.Description : Argument.Description;
+            }
 
-                var arg = "--" + opt.LongName;
-                if (opt.ShortName != default(char))
+            public override string ToString()
+            {
+                if (Positional != null)
                 {
-                    arg = String.Format("-{0}, {1}", opt.ShortName, arg);
+                    return "  " + (Positional.Required ? $"<{Positional.Name}>" : $"[{Positional.Name}]");
+                }
+                else
+                {
+                    var result = "  ";
+                    if (Argument.ShortName != default(char))
+                    {
+                        result += $"-{Argument.ShortName}, ";
+                    }
+                    result += $"--{Argument.LongName}";
+                    return result;
                 }
-                arg = "  " + arg;
-
-                optList.Add(arg);
-
-                if (arg.Length > width)
-                    width = arg.Length;
             }
+        }
 
-            width += 3;
+        /// <summary>
+        /// Converts the ArgumentCollection to a help-string.
+        /// </summary>
+        /// <returns></returns>
+        public override string ToString()
+        {
+            StringBuilder output = new StringBuilder();
+            var wrappers = new List<OptionWrapper>();
 
-            // Do the actual formatting (option + description)
-            int i = 0;
-            foreach (var option in this)
+            if (UnnamedArgumentData?.Any() == true)
             {
-                var opt = option.Value;
-
-                if (opt.IsVisible == false)
-                    continue;
+                foreach (var u in UnnamedArgumentData)
+                {
+                    if (u.GetAttribute<UnnamedCommandLineArgument>() is { } a)
+                    {
+                        wrappers.Add(new OptionWrapper(a));
+                    }
+                }
+            }
+            wrappers.AddRange(this.Values.Where(v => v.IsVisible).Select(k => new OptionWrapper(k)));
 
-                var arg =  optList[i++];
+            // Compute the options' width
+            int width = wrappers.Select(w => w.ToString().Length).Max() + 3;
 
-                if (!string.IsNullOrEmpty(opt.Description))
-                    foreach (var descSplit in opt.Description.Split('\n'))
+            foreach (var wrapper in wrappers)
+            {
+                var arg = wrapper.ToString();
+                var description = wrapper.Description();
+                if (!string.IsNullOrEmpty(description))
+                {
+                    output.Append(arg);
+                    // Offst by the arument length in the first iteration
+                    var offset = arg.Length;
+                    foreach (var descSplit in description.Split('\n'))
                     {
-                        arg = arg + new String(' ', width - arg.Length) + descSplit;
-                        output.AppendLine(arg);
-                        arg = "";
+                        output.AppendLine(descSplit.PadLeft(descSplit.Length + width - offset));
+                        offset = 0;
                     }
+                }
                 else
+                {
                     output.AppendLine(arg);
+                }
             }
             return output.ToString();
         }