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

Add support for bool, strings and arrays in Configuration settings #3135

Merged
merged 8 commits into from
Apr 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
101 changes: 69 additions & 32 deletions src/AppInstallerCLICore/Workflows/ConfigurationFlow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ namespace AppInstaller::CLI::Workflow
out << ' ' << Utility::ConvertToUTF8(property.GetString()) << '\n';
break;
case PropertyType::Boolean:
out << ' ' << (property.GetBoolean() ? "true" : "false") << '\n';
out << ' ' << (property.GetBoolean() ? Utility::LocIndView("true") : Utility::LocIndView("false")) << '\n';
break;
case PropertyType::Int64:
out << ' ' << property.GetInt64() << '\n';
Expand All @@ -115,24 +115,81 @@ namespace AppInstaller::CLI::Workflow
}
}

void OutputValueSet(OutputStream& out, const ValueSet& valueSet, size_t indent, bool isArray)
void OutputValueSet(OutputStream& out, const ValueSet& valueSet, size_t indent);

void OutputValueSetAsArray(OutputStream& out, const ValueSet& valueSetArray, size_t indent)
{
Utility::LocIndString indentString{ std::string(indent, ' ') };
bool processedFirstElement = false;

for (const auto& value : valueSet)
std::vector<IKeyValuePair<winrt::hstring, winrt::Windows::Foundation::IInspectable>> arrayValues;
for (const auto& arrayValue : valueSetArray)
{
if (isArray && !processedFirstElement)
if (arrayValue.Key() != L"treatAsArray")
{
Utility::LocIndString firstArrayIndentString{ std::string(indent - 2, ' ') };
out << firstArrayIndentString << "- " << Utility::ConvertToUTF8(value.Key()) << ':';
arrayValues.emplace_back(arrayValue);
}
}

std::sort(
arrayValues.begin(),
arrayValues.end(),
[](const IKeyValuePair<winrt::hstring, winrt::Windows::Foundation::IInspectable>& a, const IKeyValuePair<winrt::hstring, winrt::Windows::Foundation::IInspectable>& b)
{
return a.Key() < b.Key();
Copy link
Member

Choose a reason for hiding this comment

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

These keys are strings and won't sort numerically.

});

for (const auto& arrayValue : arrayValues)
{
auto arrayObject = arrayValue.Value();
IPropertyValue arrayProperty = arrayObject.try_as<IPropertyValue>();

out << indentString << "-";
if (arrayProperty)
{
OutputPropertyValue(out, arrayProperty);
}
else
{
out << indentString << Utility::ConvertToUTF8(value.Key()) << ':';
ValueSet arraySubset = arrayObject.as<ValueSet>();
auto size = arraySubset.Size();
if (size > 0)
{
// First one is special.
auto first = arraySubset.First().Current();
out << ' ' << Utility::ConvertToUTF8(first.Key()) << ':';

auto object = first.Value();
IPropertyValue property = object.try_as<IPropertyValue>();
if (property)
{
OutputPropertyValue(out, property);
}
else
{
// If not an IPropertyValue, it must be a ValueSet
ValueSet subset = object.as<ValueSet>();
out << '\n';
OutputValueSet(out, subset, indent + 4);
}

if (size > 1)
{
arraySubset.Remove(first.Key());
OutputValueSet(out, arraySubset, indent + 2);
arraySubset.Insert(first.Key(), first.Value());
}
}
}
}
}

processedFirstElement = true;
void OutputValueSet(OutputStream& out, const ValueSet& valueSet, size_t indent)
{
Utility::LocIndString indentString{ std::string(indent, ' ') };

for (const auto& value : valueSet)
{
out << indentString << Utility::ConvertToUTF8(value.Key()) << ':';

auto object = value.Value();

Expand All @@ -145,34 +202,14 @@ namespace AppInstaller::CLI::Workflow
{
// If not an IPropertyValue, it must be a ValueSet
ValueSet subset = object.as<ValueSet>();

out << '\n';
if (subset.HasKey(L"treatAsArray"))
msftrubengu marked this conversation as resolved.
Show resolved Hide resolved
{
size_t arrayIndent = indent + 2;
Utility::LocIndString arrayIndentString{ std::string(arrayIndent, ' ') };
for (const auto& arrayValue : subset)
{
if (arrayValue.Key() != L"treatAsArray")
{
auto arrayObject = arrayValue.Value();
IPropertyValue arrayProperty = arrayObject.try_as<IPropertyValue>();
if (arrayProperty)
{
out << arrayIndentString << "-";
OutputPropertyValue(out, arrayProperty);
}
else
{
ValueSet arraySubset = arrayObject.as<ValueSet>();
OutputValueSet(out, arraySubset, arrayIndent + 2, true);
}
}
}
OutputValueSetAsArray(out, subset, indent + 2);
}
else
{
OutputValueSet(out, subset, indent + 2, false);
OutputValueSet(out, subset, indent + 2);
}
}
}
Expand Down Expand Up @@ -310,7 +347,7 @@ namespace AppInstaller::CLI::Workflow
if (settings.Size() > 0)
{
out << " "_liv << Resource::String::ConfigurationSettings << '\n';
OutputValueSet(out, settings, 4, false);
OutputValueSet(out, settings, 4);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/AppInstallerCLITests/TestData/Node-Types.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ StringTrue: 'true'
BooleanFalse: false
StringFalse: 'false'

LocalTag: !mytag value
LocalTag: !myTag value
26 changes: 26 additions & 0 deletions src/AppInstallerSharedLib/AppInstallerStrings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,32 @@ namespace AppInstaller::Utility
return result;
}

std::optional<std::wstring> TryConvertToUTF16(std::string_view input, UINT codePage)
{
if (input.empty())
{
return {};
Copy link
Member

Choose a reason for hiding this comment

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

This returns an empty optional, which isn't what I would expect. It should return an optional with an empty string in it.

}

int utf16CharCount = MultiByteToWideChar(codePage, 0, input.data(), wil::safe_cast<int>(input.length()), nullptr, 0);
if (utf16CharCount == 0)
{
return {};
}

// Since the string view should not contain the null char, the result won't either.
// This allows us to use the resulting size value directly in the string constructor.
std::wstring result(wil::safe_cast<size_t>(utf16CharCount), L'\0');

int utf16CharsWritten = MultiByteToWideChar(codePage, 0, input.data(), wil::safe_cast<int>(input.length()), &result[0], wil::safe_cast<int>(result.size()));
if (utf16CharCount != utf16CharsWritten)
{
return {};
}

return std::optional{ result };
}

std::u32string ConvertToUTF32(std::string_view input)
{
if (input.empty())
Expand Down
3 changes: 3 additions & 0 deletions src/AppInstallerSharedLib/Public/AppInstallerStrings.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ namespace AppInstaller::Utility
// Converts the given UTF8 string to UTF16
std::wstring ConvertToUTF16(std::string_view input, UINT codePage = CP_UTF8);

// Tries to convert the given UTF8 string to UTF16
std::optional<std::wstring> TryConvertToUTF16(std::string_view input, UINT codePage = CP_UTF8);

// Converts the given UTF8 string to UTF32
std::u32string ConvertToUTF32(std::string_view input);

Expand Down
17 changes: 17 additions & 0 deletions src/AppInstallerSharedLib/Public/winget/Yaml.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ namespace AppInstaller::YAML
return as_dispatch(t);
}

template <typename T>
std::optional<T> try_as() const
{
Require(Type::Scalar);
Copy link
Member

Choose a reason for hiding this comment

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

This will throw if the node isn't a scalar. Should it just return an empty optional instead?

T* t = nullptr;
return try_as_dispatch(t);
}

bool operator<(const Node& other) const;

// Gets a child node from the mapping by its name.
Expand Down Expand Up @@ -158,10 +166,19 @@ namespace AppInstaller::YAML

// The workers for the as function.
std::string as_dispatch(std::string*) const;
std::optional<std::string> try_as_dispatch(std::string*) const;

std::wstring as_dispatch(std::wstring*) const;
std::optional<std::wstring> try_as_dispatch(std::wstring*) const;

int64_t as_dispatch(int64_t*) const;
std::optional<int64_t> try_as_dispatch(int64_t*) const;

int as_dispatch(int*) const;
std::optional<int> try_as_dispatch(int*) const;

bool as_dispatch(bool*) const;
std::optional<bool> try_as_dispatch(bool*) const;

Type m_type;
std::string m_tag;
Expand Down
77 changes: 60 additions & 17 deletions src/AppInstallerSharedLib/Yaml.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,22 +207,19 @@ namespace AppInstaller::YAML
{
// Integer
// 0 | -? [1-9] [0-9]*
try
auto tryInt = this->try_as<int64_t>();
if (tryInt.has_value())
{
this->as<int64_t>();
m_tagType = TagType::Int;
return;
}
catch (...)
{
}

// Either 'true' or 'false'
// Avoid THROW_HR to don't log.
if (Utility::CaseInsensitiveEquals(m_scalar, "true") ||
Utility::CaseInsensitiveEquals(m_scalar, "false"))
else
Copy link
Member

Choose a reason for hiding this comment

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

I would continue to use the early return pattern rather than an else.

{
m_tagType = TagType::Bool;
// Boolean. Either 'true' or 'false'
auto tryBool = this->try_as<bool>();
if (tryBool.has_value())
{
m_tagType = TagType::Bool;
}
}
}
}
Expand Down Expand Up @@ -319,38 +316,84 @@ namespace AppInstaller::YAML
return m_scalar;
}

std::optional<std::string> Node::try_as_dispatch(std::string*) const
{
return std::optional{ m_scalar };
}

std::wstring Node::as_dispatch(std::wstring*) const
{
return Utility::ConvertToUTF16(m_scalar);
}

std::optional<std::wstring> Node::try_as_dispatch(std::wstring*) const
{
return Utility::TryConvertToUTF16(m_scalar);
}

int64_t Node::as_dispatch(int64_t*) const
{
return std::stoll(m_scalar);
}

std::optional<int64_t> Node::try_as_dispatch(int64_t*) const
{
try
{
return std::optional{ std::stoll(m_scalar) };
}
catch(...)
{
return {};
}
}

int Node::as_dispatch(int*) const
{
// To allow HResult representation
return static_cast<int>(std::stoll(m_scalar, 0, 0));
}

bool Node::as_dispatch(bool*) const
std::optional<int> Node::try_as_dispatch(int*) const
{
if (Utility::CaseInsensitiveEquals(m_scalar, "true"))
try
{
return true;
return std::optional{ static_cast<int>(std::stoll(m_scalar, 0, 0)) };
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return std::optional{ static_cast<int>(std::stoll(m_scalar, 0, 0)) };
return std::optional{ static_cast<int>(std::stol(m_scalar, 0, 0)) };

}
else if (Utility::CaseInsensitiveEquals(m_scalar, "false"))
catch (...)
{
return {};
}
}

bool Node::as_dispatch(bool*) const
{
bool* t = nullptr;
auto tryToBool = this->try_as_dispatch(t);
if (tryToBool.has_value())
{
return false;
return tryToBool.value();
}
else
{
THROW_HR(APPINSTALLER_CLI_ERROR_YAML_INVALID_DATA);
}
}

std::optional<bool> Node::try_as_dispatch(bool*) const
{
if (Utility::CaseInsensitiveEquals(m_scalar, "true"))
{
return std::optional{ true };
}
else if (Utility::CaseInsensitiveEquals(m_scalar, "false"))
{
return std::optional{ false };
}

return {};
}

Node Load(std::string_view input)
{
Wrapper::Parser parser(input);
Expand Down
Loading