Make Profile a WinRT object

[carlos-zamora]

Profile is now a WinRT object in the TerminalApp project.

As with ColorScheme, all of the serialization logic is not exposed via
the idl. TerminalSetingsModel will handle it when it's all moved over.

I removed the "Get" and "Set" prefixes from all of the Profile
functions. It just makes more sense to use the GETSET_PROPERTY macro
to do most of the work for us.

CloseOnExitMode is now an enum off of the Profile.idl.

std::optional<wstring> got converted to hstring (as opposed to
IReference<hstring>). IReference<hstring> is not valid to MIDL.

References

#7141 - Profile is a settings object
#885 - this new settings object will be moved to a new TerminalSettingsModel project

Validation Steps Performed

• Tests passed
• Deployment succeeded

Closes #7435

[DHowett]

It will be important to retarget this PR before you merge and delete the destination branch (!)

[carlos-zamora]

Found a few small typos with massive consequences. Fixed though :)

Now I just have to figure out this one:

[zadjii-msft]

Okay I'm at 24/29, and i bet you can guess which 5 I haven't been to yet

[zadjii-msft]

this could still be auto, right? There should be an magic GUID->winrt::guid conversion already

[carlos-zamora]

I wish. I think that the compiler gets confused with the winrt::Windows::Foundation::IReference<winrt::guid>. :(

1>D:\Terminal\src\cascadia\TerminalApp\DefaultProfileUtils.cpp(22,55): error C2440: 'initializing': cannot convert from 'initializer list' to 'winrt::TerminalApp::Profile'
1>D:\Terminal\src\cascadia\TerminalApp\DefaultProfileUtils.cpp(22,55): message : No constructor could take the source type, or constructor overload resolution was ambiguous
1>D:\Terminal\src\cascadia\TerminalApp\DefaultProfileUtils.cpp(24,32): error C2664: 'auto winrt::impl::consume_TerminalApp_IProfile<winrt::TerminalApp::IProfile>::Guid(const winrt::Windows::Foundation::IReference<winrt::guid> &) const': cannot convert argument 1 from 'const GUID' to 'const winrt::Windows::Foundation::IReference<winrt::guid> &'
1>D:\Terminal\src\cascadia\TerminalApp\DefaultProfileUtils.cpp(24,32): message : Reason: cannot convert from 'const GUID' to 'const winrt::Windows::Foundation::IReference<winrt::guid>'
1>D:\Terminal\src\cascadia\TerminalApp\DefaultProfileUtils.cpp(24,21): message : No user-defined-conversion operator available that can perform this conversion, or the operator cannot be called

[carlos-zamora]

This actually seems to be the source of a lot of the comments you made. I've been getting around it by just using winrt::guid instead of GUID, but if you have a better idea in mind, go for it.

[zadjii-msft]

oh that makes me unreasonably angry

[DHowett]

something something type covariance

[carlos-zamora]

@DHowett, you might be interested in 03f6231. Is there a cleaner way to handle this issue with JsonUtils? I made it look a bit messier than before.

[carlos-zamora]

@DHowett, you might be interested in 03f6231. Is there a cleaner way to handle this issue with JsonUtils? I made it look a bit messier than before.

Chatted offline. JsonUtils should be able to handle IReference as a std::optional and uses the same converter between GUID and winrt::guid.

But it looks like IReference<winrt::guid> still gets confused somehow. Check this out:

1>D:\Terminal\src\cascadia\TerminalApp\JsonUtils.h(450,1): error C2679: binary '=': no operator found which takes a right-hand operand of type 'GUID' (or there is no acceptable conversion)
1>D:\Terminal\src\cascadia\TerminalApp\lib\Generated Files\winrt\impl\Windows.Foundation.1.h(156,1): message : could be 'winrt::Windows::Foundation::IReference<winrt::guid> &winrt::Windows::Foundation::IReference<winrt::guid>::operator =(winrt::Windows::Foundation::IReference<winrt::guid> &&)'
1>D:\Terminal\src\cascadia\TerminalApp\lib\Generated Files\winrt\impl\Windows.Foundation.1.h(156,1): message : or       'winrt::Windows::Foundation::IReference<winrt::guid> &winrt::Windows::Foundation::IReference<winrt::guid>::operator =(const winrt::Windows::Foundation::IReference<winrt::guid> &)'
1>D:\Terminal\src\cascadia\TerminalApp\JsonUtils.h(450,1): message : while trying to match the argument list '(T, GUID)'
1>        with
1>        [
1>            T=winrt::Windows::Foundation::IReference<winrt::guid>
1>        ]
1>D:\Terminal\src\cascadia\TerminalApp\JsonUtils.h(473): message : see reference to function template instantiation 'bool TerminalApp::JsonUtils::GetValue<T,_Ty>(const Json::Value &,T &,Converter &&)' being compiled
1>        with
1>        [
1>            T=winrt::Windows::Foundation::IReference<winrt::guid>,
1>            _Ty=TerminalApp::JsonUtils::ConversionTrait<winrt::guid>,
1>            Converter=TerminalApp::JsonUtils::ConversionTrait<winrt::guid>
1>        ]
1>D:\Terminal\src\cascadia\TerminalApp\JsonUtils.h(513): message : see reference to function template instantiation 'bool TerminalApp::JsonUtils::GetValueForKey<T,TerminalApp::JsonUtils::ConversionTrait<winrt::guid>>(const Json::Value &,std::string_view,T &,Converter &&)' being compiled
1>        with
1>        [
1>            T=winrt::Windows::Foundation::IReference<winrt::guid>,
1>            Converter=TerminalApp::JsonUtils::ConversionTrait<winrt::guid>
1>        ]
1>D:\Terminal\src\cascadia\TerminalApp\Profile.cpp(322): message : see reference to function template instantiation 'bool TerminalApp::JsonUtils::GetValueForKey<winrt::Windows::Foundation::IReference<winrt::guid>>(const Json::Value &,std::string_view,T &)' being compiled
1>        with
1>        [
1>            T=winrt::Windows::Foundation::IReference<winrt::guid>
1>        ]

[zadjii-msft]

re:

I was considering that for a bit, actually. If we go that route, I think it would make sense to break up

GETSET_PROPERTY(Windows::Foundation::IReference<guid>, Guid, nullptr);

into

// return/accept guid instead of IReference<guid>
bool HasGuid();
guid Guid();
void Guid(guid);

Honestly, that feels better to me, but I know how much annoying work that is. IMO, it seems like external to the deserialization of the settings, a profile should always have a GUID. Kinda like how right now we just throw if a profile ever doesn't. That'll prevent the need for all the other classes to do the if (profile.Guid() != nullptr) { profile.Guid().Value(); } dance.

[zadjii-msft]

Alright, these are all only nits, so I won't block over them.

[miniksa]

I don't feel confident enough in WinRT-isms to sign off here, but I didn't see a whole lot out of order. Just left a few small comments.

[DHowett]

NAK because of the null discussion and change in behavior

[carlos-zamora]

I still want/need to double check if there is similar behavior with making the other settings null. Assigning to myself to do that. So this PR is not ready to go yet.

[carlos-zamora]

The latest commit closes #7435 as well.

[DHowett]

@msftbot merge this in 1 minute

[ghost]

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

• I won't merge this pull request until after the UTC date Fri, 28 Aug 2020 01:08:45 GMT, which is in 1 minute

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

[ghost]

🎉Windows Terminal Preview v1.4.2652.0 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Store Download