Skip to content

Commit

Permalink
JsonUtils: remove Optional specializations, use constexpr if, add IRe…
Browse files Browse the repository at this point in the history 
…ference
  • Loading branch information
@DHowett
DHowett committed Jun 19, 2020
1 parent f5d1a6b commit 6122eb8
Showing 1 changed file with 45 additions and 34 deletions.
79 changes: 45 additions & 34 deletions src/cascadia/TerminalApp/JsonUtilsNew.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,15 @@ Author(s):

namespace winrt
{
// If we don't use winrt, nobody will include the ConversionTrait for winrt::guid.
// If nobody includes it, this forward declaration will suffice.
// If we don't use winrt, nobody will include the ConversionTraits for winrt stuff.
// If nobody includes it, these forward declarations will suffice.
struct guid;
struct hstring;
namespace Windows::Foundation
{
template<typename T>
struct IReference;
}
}

namespace TerminalApp::JsonUtils
Expand All @@ -45,12 +51,21 @@ namespace TerminalApp::JsonUtils
struct DeduceOptional
{
using Type = typename std::decay<T>::type;
static constexpr bool IsOptional = false;
};

template<typename TOpt>
struct DeduceOptional<std::optional<TOpt>>
{
using Type = typename std::decay<TOpt>::type;
static constexpr bool IsOptional = true;
};

template<typename TOpt>
struct DeduceOptional<::winrt::Windows::Foundation::IReference<TOpt>>
{
using Type = typename std::decay<TOpt>::type;
static constexpr bool IsOptional = true;
};
}

Expand Down Expand Up @@ -105,7 +120,9 @@ namespace TerminalApp::JsonUtils
template<typename T>
struct ConversionTrait
{
// FromJson, CanConvert are not defined so as to cause a compile error (which forces a specialization)
// Forward-declare these so the linker can pick up specializations from elsewhere!
T FromJson(const Json::Value&);
bool CanConvert(const Json::Value& json);
};

template<>
Expand Down Expand Up @@ -136,6 +153,18 @@ namespace TerminalApp::JsonUtils
}
};

#ifdef WINRT_BASE_H
template<>
struct ConversionTrait<winrt::hstring> : public ConversionTrait<std::wstring>
{
// Leverage the wstring converter's validation
winrt::hstring FromJson(const Json::Value& json)
{
return winrt::hstring{ til::u8u16(Detail::GetStringView(json)) };
}
};
#endif

template<>
struct ConversionTrait<bool>
{
Expand Down Expand Up @@ -248,7 +277,7 @@ namespace TerminalApp::JsonUtils
}

const auto string{ Detail::GetStringView(json) };
return (string.length() == 7 || string.length() == 3) && string.front() == '#';
return (string.length() == 7 || string.length() == 4) && string.front() == '#';
}
};

Expand Down Expand Up @@ -334,6 +363,18 @@ namespace TerminalApp::JsonUtils
template<typename T, typename Converter>
bool GetValue(const Json::Value& json, T& target, Converter&& conv)
{
if constexpr (Detail::DeduceOptional<T>::IsOptional)
{
// FOR OPTION TYPES
// - If the json object is set to `null`, then
// we'll instead set the target back to the empty optional.
if (json.isNull())
{
target = T{}; // zero-construct an empty optional
return true;
}
}

if (json)
{
if (!conv.CanConvert(json))
Expand All @@ -347,36 +388,6 @@ namespace TerminalApp::JsonUtils
return false;
}

// Method Description:
// - Overload on GetValue that will populate a std::optional with a value converted from json
// - If the json value doesn't exist we'll leave the target object unmodified.
// - If the json object is set to `null`, then
// we'll instead set the target back to nullopt.
// Arguments:
// - json: the json object to convert
// - target: the value to populate with the converted result
// Return Value:
// - a boolean indicating whether the optional was changed
//
// GetValue, type-deduced for optional, manual converter
template<typename TOpt, typename Converter>
bool GetValue(const Json::Value& json, std::optional<TOpt>& target, Converter&& conv)
{
if (json.isNull())
{
target = std::nullopt;
return true; // null is valid for optionals
}

std::decay_t<TOpt> local{};
if (GetValue(json, local, std::forward<Converter>(conv)))
{
target = std::move(local);
return true;
}
return false;
}

// GetValue, forced return type, manual converter
template<typename T, typename Converter>
std::decay_t<T> GetValue(const Json::Value& json, Converter&& conv)
Expand Down

1 comment on commit 6122eb8

@github-actions
Copy link

Choose a reason for hiding this comment

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

New misspellings found, please review:

  • JU
To accept these changes, run the following commands 
perl -e '
my @expect_files=qw('".github/actions/spell-check/expect/alphabet.txt
.github/actions/spell-check/expect/expect.txt
.github/actions/spell-check/expect/web.txt"');
@ARGV=@expect_files;
my @stale=qw('"atg BKMK consoleaccessibility FFF hyperlink NRCS remoting SCS shobjidl Tofill xab xb xbc xca xce xdd xffff "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/expect/6122eb82612002a9608084cd339bc886c3540aa6.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"JU nrcs Remoting Scs Shobjidl "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/expect || echo '... you want to ensure .github/actions/spell-check/expect/6122eb82612002a9608084cd339bc886c3540aa6.txt is added to your repository...'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. You can copy the contents of each perl command excluding the outer ' marks and dropping any '"/"' quotation mark pairs into a file and then run perl file.pl from the root of the repository to run the code. Alternatively, you can manually insert the items...

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spell-check/dictionary/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spell-check/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The :check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

Please sign in to comment.