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.

Sign up for GitHub

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

Enable long aliases #2107

Merged
merged 6 commits into from
Jun 16, 2022
Merged

Enable long aliases #2107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
57cfa21
Add member variable for alternate names
Trenly May 12, 2022
41e8070
Address PR Comments
Trenly May 17, 2022
e1ac676
Remove confusing alternate name
Trenly Jun 7, 2022
1f3b483
Fix Collision Check
Trenly Jun 7, 2022
e4d7a1a
Revert change to expect.txt
Trenly Jun 7, 2022
b31c7a9
Fix Test
Trenly Jun 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/allow.txt
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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