Skip to content

Commit

Permalink
Enable long aliases (#2107)
Browse files Browse the repository at this point in the history 
This change allows for argument aliases to be more than 1 character
  • Loading branch information
@Trenly
Trenly authored Jun 16, 2022
1 parent 7d86fc0 commit 5bdc55e
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 9 deletions.
1 change: 1 addition & 0 deletions .github/actions/spelling/allow.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ cla
CLASSNOTAVAILABLE
CLSCTX
cmake
cmd
cmdlet
cmdlets
cmp
Expand Down
8 changes: 4 additions & 4 deletions src/AppInstallerCLICore/Argument.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace AppInstaller::CLI
case Args::Type::Tag:
return Argument{ "tag", NoAlias, Args::Type::Tag, Resource::String::TagArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help };
case Args::Type::Command:
return Argument{ "command", NoAlias, Args::Type::Command, Resource::String::CommandArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help };
return Argument{ "command", NoAlias, "cmd", Args::Type::Command, Resource::String::CommandArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help };
case Args::Type::Source:
return Argument{ "source", 's', Args::Type::Source, Resource::String::SourceArgumentDescription, ArgumentType::Standard };
case Args::Type::DependencySource:
Expand All @@ -54,9 +54,9 @@ namespace AppInstaller::CLI
case Args::Type::InstallLocation:
return Argument{ "location", 'l', Args::Type::InstallLocation, Resource::String::LocationArgumentDescription, ArgumentType::Standard };
case Args::Type::HashOverride:
return Argument{ "force", Argument::NoAlias, Args::Type::HashOverride, Resource::String::InstallForceArgumentDescription, ArgumentType::Flag, Settings::TogglePolicy::Policy::HashOverride };
return Argument{ "force", NoAlias, Args::Type::HashOverride, Resource::String::InstallForceArgumentDescription, ArgumentType::Flag, Settings::TogglePolicy::Policy::HashOverride };
case Args::Type::AcceptPackageAgreements:
return Argument{ "accept-package-agreements", Argument::NoAlias, Args::Type::AcceptPackageAgreements, Resource::String::AcceptPackageAgreementsArgumentDescription, ArgumentType::Flag };
return Argument{ "accept-package-agreements", NoAlias, Args::Type::AcceptPackageAgreements, Resource::String::AcceptPackageAgreementsArgumentDescription, ArgumentType::Flag };
case Args::Type::HashFile:
return Argument{ "file", 'f', Args::Type::HashFile, Resource::String::FileArgumentDescription, ArgumentType::Positional, true };
case Args::Type::Msix:
Expand All @@ -80,7 +80,7 @@ namespace AppInstaller::CLI
case Args::Type::RetroStyle:
return Argument{ "retro", NoAlias, Args::Type::RetroStyle, Resource::String::RetroArgumentDescription, ArgumentType::Flag, Argument::Visibility::Hidden };
case Args::Type::VerboseLogs:
return Argument{ "verbose-logs", NoAlias, Args::Type::VerboseLogs, Resource::String::VerboseLogsArgumentDescription, ArgumentType::Flag };
return Argument{ "verbose-logs", NoAlias, "verbose", Args::Type::VerboseLogs, Resource::String::VerboseLogsArgumentDescription, ArgumentType::Flag};
case Args::Type::CustomHeader:
return Argument{ "header", NoAlias, Args::Type::CustomHeader, Resource::String::HeaderArgumentDescription, ArgumentType::Standard, Argument::Visibility::Help };
case Args::Type::AcceptSourceAgreements:
Expand Down
47 changes: 47 additions & 0 deletions src/AppInstallerCLICore/Argument.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ namespace AppInstaller::CLI
// Defines an argument with no alias.
constexpr static char NoAlias = '\0';

// Defines an argument with no alternate name
constexpr static std::string_view NoAlternateName = "";

Argument(std::string_view name, char alias, Execution::Args::Type execArgType, Resource::StringId desc) :
m_name(name), m_alias(alias), m_execArgType(execArgType), m_desc(std::move(desc)) {}

Expand All @@ -67,6 +70,24 @@ namespace AppInstaller::CLI
Argument(std::string_view name, char alias, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Argument::Visibility visibility, bool required) :
m_name(name), m_alias(alias), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_visibility(visibility), m_required(required) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, bool required) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_required(required) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Argument::Visibility visibility) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_visibility(visibility) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, bool required) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_required(required) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Argument::Visibility visibility, bool required) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_visibility(visibility), m_required(required) {}

~Argument() = default;

Argument(const Argument&) = default;
Expand All @@ -89,6 +110,7 @@ namespace AppInstaller::CLI
// Arguments are not localized at this time.
Utility::LocIndView Name() const { return Utility::LocIndView{ m_name }; }
char Alias() const { return m_alias; }
std::string_view AlternateName() const { return m_alternateName; }
Execution::Args::Type ExecArgType() const { return m_execArgType; }
const Resource::StringId& Description() const { return m_desc; }
bool Required() const { return m_required; }
Expand Down Expand Up @@ -128,8 +150,33 @@ namespace AppInstaller::CLI
Argument(std::string_view name, char alias, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Argument::Visibility visibility, Settings::TogglePolicy::Policy groupPolicy, Settings::AdminSetting adminSetting) :
m_name(name), m_alias(alias), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_visibility(visibility), m_groupPolicy(groupPolicy), m_adminSetting(adminSetting) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, Settings::ExperimentalFeature::Feature feature) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_feature(feature) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, bool required, Settings::ExperimentalFeature::Feature feature) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_required(required), m_feature(feature) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Settings::ExperimentalFeature::Feature feature) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_feature(feature) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Argument::Visibility visibility, Settings::ExperimentalFeature::Feature feature) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_visibility(visibility), m_feature(feature) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, bool required, Settings::ExperimentalFeature::Feature feature) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_required(required), m_feature(feature) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Argument::Visibility visibility, bool required, Settings::ExperimentalFeature::Feature feature) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_visibility(visibility), m_required(required), m_feature(feature) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Settings::TogglePolicy::Policy groupPolicy) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_groupPolicy(groupPolicy) {}

Argument(std::string_view name, char alias, std::string_view alternateName, Execution::Args::Type execArgType, Resource::StringId desc, ArgumentType type, Argument::Visibility visibility, Settings::TogglePolicy::Policy groupPolicy, Settings::AdminSetting adminSetting) :
m_name(name), m_alias(alias), m_alternateName(alternateName), m_execArgType(execArgType), m_desc(std::move(desc)), m_type(type), m_visibility(visibility), m_groupPolicy(groupPolicy), m_adminSetting(adminSetting) {}

std::string_view m_name;
char m_alias;
std::string_view m_alternateName;
Execution::Args::Type m_execArgType;
Resource::StringId m_desc;
bool m_required = false;
Expand Down
11 changes: 9 additions & 2 deletions src/AppInstallerCLICore/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,9 @@ namespace AppInstaller::CLI
{
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.Alias() << ',';
}
if (arg.AlternateName() != Argument::NoAlternateName) {
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.AlternateName() << ',';
}
strstr << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR << arg.Name();

argNames.emplace_back(strstr.str());
Expand Down Expand Up @@ -549,7 +552,8 @@ namespace AppInstaller::CLI
{
// This is an arg name, find it and process its value if needed.
// Skip the double arg identifier chars.
std::string_view argName = currArg.substr(2);
size_t argStart = currArg.find_first_not_of(APPINSTALLER_CLI_ARGUMENT_IDENTIFIER_CHAR);
std::string_view argName = currArg.substr(argStart);
bool argFound = false;

bool hasValue = false;
Expand All @@ -564,7 +568,10 @@ namespace AppInstaller::CLI

for (const auto& arg : m_arguments)
{
if (Utility::CaseInsensitiveEquals(argName, arg.Name()))
if (
Utility::CaseInsensitiveEquals(argName, arg.Name()) ||
Utility::CaseInsensitiveEquals(argName, arg.AlternateName())
)
{
if (arg.Type() == ArgumentType::Flag)
{
Expand Down
46 changes: 43 additions & 3 deletions src/AppInstallerCLITests/Command.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ std::string GetArgumentName(const Argument& arg)
return std::string{ arg.Name() };
}

std::string GetArgumentAlternateName(const Argument& arg)
{
if (arg.AlternateName() == Argument::NoAlternateName)
{
return {};
}
else
{
return std::string{ arg.AlternateName() };
}
}

std::string GetArgumentAlias(const Argument& arg)
{
if (arg.Alias() == Argument::NoAlias)
Expand All @@ -36,10 +48,9 @@ std::string GetArgumentAlias(const Argument& arg)
}

template <typename Enumerable, typename Op>
void EnsureStringsAreLowercaseAndNoCollisions(const std::string& info, const Enumerable& e, Op& op, bool requireLower = true)
void EnsureStringsAreLowercaseAndNoCollisions(const std::string& info, const Enumerable& e, Op& op, std::unordered_set<std::string>& values, bool requireLower = true)
{
INFO(info);
std::unordered_set<std::string> values;

for (const auto& val : e)
{
Expand All @@ -62,13 +73,25 @@ void EnsureStringsAreLowercaseAndNoCollisions(const std::string& info, const Enu
}
}

template <typename Enumerable, typename Op>
void EnsureStringsAreLowercaseAndNoCollisions(const std::string& info, const Enumerable& e, Op& op, bool requireLower = true)
{
std::unordered_set<std::string> values;
EnsureStringsAreLowercaseAndNoCollisions(info, e, op, values, requireLower);
}

void EnsureCommandConsistency(const Command& command)
{
EnsureStringsAreLowercaseAndNoCollisions(command.FullName() + " commands", command.GetCommands(), GetCommandName);

auto args = command.GetArguments();
Argument::GetCommon(args);
EnsureStringsAreLowercaseAndNoCollisions(command.FullName() + " argument names", args, GetArgumentName);

// Argument names and alternate names exist in the same space, so both need to be checked as a set
std::unordered_set<std::string> allArgumentNames;
EnsureStringsAreLowercaseAndNoCollisions(command.FullName() + " argument names", args, GetArgumentName, allArgumentNames);
EnsureStringsAreLowercaseAndNoCollisions(command.FullName() + " argument alternate name", args, GetArgumentAlternateName, allArgumentNames);
// Argument aliases use a different space than the names and can be checked separately
EnsureStringsAreLowercaseAndNoCollisions(command.FullName() + " argument alias", args, GetArgumentAlias, false);

// No : allowed in commands
Expand Down Expand Up @@ -414,6 +437,23 @@ TEST_CASE("ParseArguments_NameWithAdjoinedValue", "[command]")
RequireValueParsedToArg(values[0].substr(7), command.m_args[0], args);
}

TEST_CASE("ParseArguments_AlternateNameWithAdjoinedValue", "[command]")
{
Args args;
TestCommand command({
Argument{ "pos1", 'p', "p1", Args::Type::Channel, DefaultDesc, ArgumentType::Positional},
Argument{ "std1", 's', Args::Type::Command, DefaultDesc, ArgumentType::Standard },
Argument{ "pos2", 'q', Args::Type::Count, DefaultDesc, ArgumentType::Positional },
});

std::vector<std::string> values{ "--p1=Val1" };
Invocation inv{ std::vector<std::string>(values) };

command.ParseArguments(inv, args);

RequireValueParsedToArg(values[0].substr(5), command.m_args[0], args);
}

TEST_CASE("ParseArguments_NameFlag", "[command]")
{
Args args;
Expand Down

0 comments on commit 5bdc55e

Please sign in to comment.