Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue #482: reorder options in auto help text #484

Merged
merged 14 commits into from
Jul 29, 2019
102 changes: 95 additions & 7 deletions src/CommandLine/Text/HelpText.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,72 @@ namespace CommandLine.Text
/// Provides means to format an help screen.
/// You can assign it in place of a <see cref="System.String"/> instance.
/// </summary>



public struct ComparableOption
{
public bool Required;
public bool IsOption;
public bool IsValue;
public string LongName;
public string ShortName;
public int Index;
}

public class HelpText
{

#region ordering

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 #region Option ordering

ComparableOption ToComparableOption(Specification spec, int index)
{
OptionSpecification option = spec as OptionSpecification;
ValueSpecification value = spec as ValueSpecification;
bool required = option?.Required ?? false;

return new ComparableOption()
{
Required = required,
IsOption = option != null,
IsValue = value != null,
LongName = option?.LongName ?? value?.MetaName,
ShortName = option?.ShortName,
Index = index
};
}


public Comparison<ComparableOption> OptionComparison { get; set; } = null;

public static Comparison<ComparableOption> RequiredThenAlphaComparison = (ComparableOption attr1, ComparableOption attr2) =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove redundant else and braces, and use StringComparison.Ordinal

       public static Comparison<ComparableOption> RequiredThenAlphaComparison  = (attr1, attr2) =>
    {
        if (attr1.IsOption && attr2.IsOption)
        {
            if (attr1.Required && !attr2.Required)
                return -1;

            if (!attr1.Required && attr2.Required)
                return 1;

            int t = string.Compare(attr1.LongName, attr2.LongName, StringComparison.Ordinal);
            return t;
        }

        return attr1.IsOption && attr2.IsValue ? -1 : 1;
    };

{
if (attr1.IsOption && attr2.IsOption)
{
if (attr1.Required && !attr2.Required)
{
return -1;
}
else if (!attr1.Required && attr2.Required)
{
return 1;
}

return String.Compare(attr1.LongName, attr2.LongName, StringComparison.Ordinal);

}
else if (attr1.IsOption && attr2.IsValue)
{
return -1;
}
else
{
return 1;
}
};

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#endregion

#endregion

private const int BuilderCapacity = 128;
private const int DefaultMaximumLength = 80; // default console width
/// <summary>
Expand Down Expand Up @@ -240,21 +304,24 @@ public SentenceBuilder SentenceBuilder
/// <param name='onExample'>A delegate used to customize <see cref="CommandLine.Text.Example"/> model used to render text block of usage examples.</param>
/// <param name="verbsIndex">If true the output style is consistent with verb commands (no dashes), otherwise it outputs options.</param>
/// <param name="maxDisplayWidth">The maximum width of the display.</param>
/// <param name="comparison">a comparison lambda to order options in help text</param>
/// <remarks>The parameter <paramref name="verbsIndex"/> is not ontly a metter of formatting, it controls whether to handle verbs or options.</remarks>
public static HelpText AutoBuild<T>(
ParserResult<T> parserResult,
Func<HelpText, HelpText> onError,
Func<Example, Example> onExample,
bool verbsIndex = false,
int maxDisplayWidth = DefaultMaximumLength)
int maxDisplayWidth = DefaultMaximumLength,
Comparison<ComparableOption> comparison = null)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//remove this line, no more parameters are used
//Comparison comparison = null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

working on it.

{
var auto = new HelpText
{
Heading = HeadingInfo.Empty,
Copyright = CopyrightInfo.Empty,
AdditionalNewLineAfterOption = true,
AddDashesToOption = !verbsIndex,
MaximumDisplayWidth = maxDisplayWidth
MaximumDisplayWidth = maxDisplayWidth,
OptionComparison = comparison
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//remove this line, no more parameters are used
// OptionComparison = comparison

};

try
Expand Down Expand Up @@ -736,14 +803,37 @@ private HelpText AddOptionsImpl(
int maximumLength)
{
var maxLength = GetMaxLength(specifications);



optionsHelp = new StringBuilder(BuilderCapacity);

var remainingSpace = maximumLength - (maxLength + TotalOptionPadding);

specifications.ForEach(
option =>
AddOption(requiredWord, maxLength, option, remainingSpace));
if (OptionComparison != null)
{
int i = -1;
var comparables = specifications.ToList().Select(s =>
{
i++;
return ToComparableOption(s, i);
}).ToList();
comparables.Sort(OptionComparison);


foreach (var comparable in comparables)
{
Specification spec = specifications.ElementAt(comparable.Index);
AddOption(requiredWord, maxLength, spec, remainingSpace);
}
}
else
{
specifications.ForEach(
option =>
AddOption(requiredWord, maxLength, option, remainingSpace));

}

return this;
}
Expand Down Expand Up @@ -953,5 +1043,3 @@ private static string FormatDefaultValue<T>(T value)

}
}


48 changes: 48 additions & 0 deletions tests/CommandLine.Tests/Fakes/OPtions_HelpText_Ordering.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2005-2015 Giacomo Stelluti Scala & Contributors. All rights reserved. See License.md in the project root for license information.

using System.Collections.Generic;
using System.Runtime.CompilerServices;

namespace CommandLine.Tests.Fakes
{

[Verb("verb1")]
class Options_HelpText_Ordering_Verb1
{
[Option('a', "alpha", Required = true)]
public string alphaOption { get; set; }

[Option('b', "alpha2", Required = true)]
public string alphaTwoOption { get; set; }

[Option('d', "charlie", Required = false)]
public string deltaOption { get; set; }

[Option('c', "bravo", Required = false)]
public string charlieOption { get; set; }

[Option('f', "foxtrot", Required = false)]
public string foxOption { get; set; }

[Option('e', "echo", Required = false)]
public string echoOption { get; set; }

[Value(0)] public string someExtraOption { get; set; }
}

[Verb("verb2")]
class Options_HelpText_Ordering_Verb2
{
[Option('a', "alpha", Required = true)]
public string alphaOption { get; set; }

[Option('b', "alpha2", Required = true)]
public string alphaTwoOption { get; set; }

[Option('c', "bravo", Required = false)]
public string charlieOption { get; set; }

[Option('d', "charlie", Required = false)]
public string deltaOption { get; set; }
}
}
197 changes: 197 additions & 0 deletions tests/CommandLine.Tests/Unit/Issue482Tests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using CommandLine.Core;
using System.Linq;
using System.Reflection;
using CommandLine.Infrastructure;
using CommandLine.Tests.Fakes;
using CommandLine.Text;
using FluentAssertions;
using Xunit;
using System.Text;
using Xunit.Sdk;

namespace CommandLine.Tests.Unit
{
public class Issue482Tests
{
[Fact]
public void AutoBuild_without_ordering()
{
string expectedCompany = "Company";


var parser = Parser.Default;
var parseResult = parser.ParseArguments<Options_HelpText_Ordering_Verb1, Options_HelpText_Ordering_Verb2>(
new[] { "verb1", "--alpha", "alpaga", "--alpha2", "alala", "--charlie", "charlot" })
.WithNotParsed(errors => { throw new InvalidOperationException("Must be parsed."); })
.WithParsed(args => {; });

var message = HelpText.AutoBuild(parseResult,
error =>
{
error.OptionComparison = HelpText.RequiredThenAlphaComparison;
return error;
},
ex => ex
);

string helpMessage = message.ToString();
var helps = helpMessage.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).Skip(2).ToList<string>();
List<string> expected = new List<string>()
{
" -a, --alpha Required.",
" -b, --alpha2 Required.",
" -d, --charlie",
" -c, --bravo",
"-f, --foxtrot",
"-e, --echo",
"--help Display this help screen.",
"--version Display version information.",
"value pos. 0"
};
Assert.Equal(expected.Count, helps.Count);
int i = 0;
foreach (var expect in expected)
{
Assert.Equal(expect.Trim(), helps[i].Trim());
i++;
}

;
}

[Fact]
public void AutoBuild_with_ordering()
{
string expectedCompany = "Company";


var parser = Parser.Default;
var parseResult = parser.ParseArguments<Options_HelpText_Ordering_Verb1, Options_HelpText_Ordering_Verb2>(
new[] { "verb1", "--alpha", "alpaga", "--alpha2", "alala", "--charlie", "charlot" })
.WithNotParsed(errors => { throw new InvalidOperationException("Must be parsed."); })
.WithParsed(args => {; });

Comparison<ComparableOption> comparison = HelpText.RequiredThenAlphaComparison;

string message = HelpText.AutoBuild(parseResult,
error =>
{
error.OptionComparison = HelpText.RequiredThenAlphaComparison;
return error;
},
ex => ex,
comparison: comparison);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//remove this line
//comparison: comparison



string helpMessage = message.ToString();
var helps = helpMessage.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).Skip(2).ToList<string>();
List<string> expected = new List<string>()
{
" -a, --alpha Required.",
" -b, --alpha2 Required.",
" -c, --bravo",
" -d, --charlie",
"-e, --echo",
"-f, --foxtrot",
"--help Display this help screen.",
"--version Display version information.",
"value pos. 0"
};
Assert.Equal(expected.Count, helps.Count);
int i = 0;
foreach (var expect in expected)
{
Assert.Equal(expect.Trim(), helps[i].Trim());
i++;
}

;
}

[Fact]
public void AutoBuild_with_ordering_on_shortName()
{
string expectedCompany = "Company";


var parser = Parser.Default;
var parseResult = parser.ParseArguments<Options_HelpText_Ordering_Verb1, Options_HelpText_Ordering_Verb2>(
new[] { "verb1", "--alpha", "alpaga", "--alpha2", "alala", "--charlie", "charlot" })
.WithNotParsed(errors => { throw new InvalidOperationException("Must be parsed."); })
.WithParsed(args => {; });

Comparison<ComparableOption> orderOnShortName = (ComparableOption attr1, ComparableOption attr2) =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove redundant else and braces, and use StringComparison.Ordinal as done here

{
if (attr1.IsOption && attr2.IsOption)
{
if (attr1.Required && !attr2.Required)
{
return -1;
}
else if (!attr1.Required && attr2.Required)
{
return 1;
}
else
{
if (string.IsNullOrEmpty(attr1.ShortName) && !string.IsNullOrEmpty(attr2.ShortName))
{
return 1;
}
else if (!string.IsNullOrEmpty(attr1.ShortName) && string.IsNullOrEmpty(attr2.ShortName))
{
return -1;
}
return String.Compare(attr1.ShortName, attr2.ShortName, StringComparison.Ordinal);
}
}
else if (attr1.IsOption && attr2.IsValue)
{
return -1;
}
else
{
return 1;
}
};

string message = HelpText.AutoBuild(parseResult,
error =>
{
error.OptionComparison = orderOnShortName;
return error;
},
ex => ex,
false,
80,
orderOnShortName);
Copy link
Collaborator

@moh-hassan moh-hassan Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

//remove the line orderOnShortName
//Enough to call without extra parameters:

        string message = HelpText.AutoBuild(parseResult,
                error =>
                {
                    error.OptionComparison = orderOnShortName;
                    return error;
                },
                ex => ex);



var helps = message.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries).Skip(2).ToList<string>();
List<string> expected = new List<string>()
{
" -a, --alpha Required.",
" -b, --alpha2 Required.",
" -c, --bravo",
" -d, --charlie",
"-e, --echo",
"-f, --foxtrot",
"--help Display this help screen.",
"--version Display version information.",
"value pos. 0"
};
Assert.Equal(expected.Count, helps.Count);
int i = 0;
foreach (var expect in expected)
{
Assert.Equal(expect.Trim(), helps[i].Trim());
i++;
}
}


}
}