User/aeloros/feedback (#1191)

aeloros · web-flow · commit 2da59de19cba · 2021-08-09T20:56:14.000Z
* pr feedback batch 1

* more feedback

* more feedback
diff --git a/dev/AppLifecycle/AppInstance.cpp b/dev/AppLifecycle/AppInstance.cpp
@@ -34,7 +34,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         // Push past the '----' commandline argument prefix.
         argsStart += 4;
 
-        auto argsEnd = commandLine.find_first_of(L" ", argsStart);
+        auto argsEnd = commandLine.find_first_of(L' ', argsStart);
 
         // Separate the argument from any behind it on the command-line.
         std::wstring argument;
@@ -53,7 +53,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         }
 
         // We explicitly use find_first_of here, so that the resulting data may contain : as a valid character.
-        auto argsDelim = argument.find_first_of(L":");
+        auto argsDelim = argument.find_first_of(L':');
         if (argsDelim == std::wstring::npos)
         {
             return { argument, L"" };
@@ -316,7 +316,6 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
     {
         ExtendedActivationKind kind = ExtendedActivationKind::Launch;
         IInspectable data;
-        bool foundArgs = false;
 
         // For packaged, try to get platform args first.
         if (HasIdentity())
@@ -325,13 +324,12 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
             {
                 data = args;
                 kind = static_cast<ExtendedActivationKind>(args.Kind());
-                foundArgs = true;
             }
         }
 
         // Handle all Windows App SDK types next (both packaged and unpackaged).
-        std::wstring commandLine = std::wstring(GetCommandLine());
-        if (!foundArgs)
+        std::wstring commandLine{ GetCommandLine() };
+        if (data == nullptr)
         {
             auto [contractArgument, contractData] = ParseCommandLine(commandLine);
 
@@ -349,10 +347,10 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
                     }
                 }
 
-                if (CompareStringOrdinal(contractArgument.c_str(), -1, c_protocolArgumentString, -1, TRUE) == CSTR_EQUAL)
+                if (CompareStringOrdinal(contractArgument.c_str(), static_cast<int>(contractArgument.size()), c_protocolArgumentString, -1, TRUE) == CSTR_EQUAL)
                 {
                     kind = ExtendedActivationKind::Protocol;
-                    auto args = make<ProtocolActivatedEventArgs>(contractData);
+                    auto args = make<ProtocolActivatedEventArgs>(contractData.c_str());
                     data = args;
 
                     // Encoded launch is a protocol launch where the argument data is
@@ -364,17 +362,15 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
                     {
                         std::tie(kind, data) = GetEncodedLaunchActivatedEventArgs(args);
                     }
-
-                    foundArgs = true;
                 }
             }
         }
 
         // All scenarios should just be marked as Launch.
-        if (!foundArgs)
+        if (data == nullptr)
         {
             kind = ExtendedActivationKind::Launch;
-            data = make<LaunchActivatedEventArgs>(commandLine);
+            data = make<LaunchActivatedEventArgs>(commandLine.c_str());
         }
 
         return make<AppActivationArguments>(kind, data);
diff --git a/dev/AppLifecycle/ExtensionContract.h b/dev/AppLifecycle/ExtensionContract.h
@@ -39,12 +39,12 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         {
             if (CompareStringOrdinal(pair.Name().c_str(), -1, c_contractIdKeyName, -1, TRUE) == CSTR_EQUAL)
             {
+                auto contractId = pair.Value().c_str();
                 for (const auto& extension : c_extensionMap)
                 {
-                    std::wstring contractId = pair.Value().c_str();
-                    if (CompareStringOrdinal(contractId.c_str(), -1, extension.contractId, -1, TRUE) == CSTR_EQUAL)
+                    if (CompareStringOrdinal(contractId, -1, extension.contractId, -1, TRUE) == CSTR_EQUAL)
                     {
-                        return { extension.kind, extension.factory(uri).as<winrt::Windows::Foundation::IInspectable>() };
+                        return { extension.kind, extension.factory(uri) };
                     }
                 }
             }
diff --git a/dev/AppLifecycle/FileActivatedEventArgs.h b/dev/AppLifecycle/FileActivatedEventArgs.h
@@ -10,13 +10,13 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
     using namespace winrt::Windows::ApplicationModel::Activation;
     using namespace winrt::Windows::Storage;
 
-    constexpr PCWSTR c_fileContractId = L"Windows.File";
+    constexpr inline PCWSTR c_fileContractId = L"Windows.File";
 
     class FileActivatedEventArgs : public winrt::implements<FileActivatedEventArgs, IFileActivatedEventArgs, ActivatedEventArgsBase,
         IInternalValueMarshalable>
     {
     public:
-        FileActivatedEventArgs(const std::wstring verb, const std::wstring file, const bool delayVerification = false)
+        FileActivatedEventArgs(const winrt::hstring verb, const winrt::hstring file, const bool delayVerification = false)
         {
             if (verb.empty())
             {
@@ -47,16 +47,17 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         static winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri)
         {
             auto query = uri.QueryParsed();
-            auto verb = std::wstring(query.GetFirstValueByName(L"Verb"));
-            auto file = std::wstring(query.GetFirstValueByName(L"File"));
+            auto verb = query.GetFirstValueByName(L"Verb");
+            auto file = query.GetFirstValueByName(L"File");
             return make<FileActivatedEventArgs>(verb, file);
         }
 
         // IInternalValueMarshalable
         winrt::Windows::Foundation::Uri Serialize()
         {
             auto uri = GenerateEncodedLaunchUri(L"App", c_fileContractId);
-            uri += L"&Verb=" + m_verb + std::wstring(L"&File=") + m_path;
+            uri += L"&Verb=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_verb.c_str());
+            uri += L"&File=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_path.c_str());
             return winrt::Windows::Foundation::Uri(uri);
         }
 
@@ -72,8 +73,8 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         }
 
     private:
-        std::wstring m_verb;
-        std::wstring m_path;
+        winrt::hstring m_verb;
+        winrt::hstring m_path;
         IVector<IStorageItem> m_files;
     };
 }
diff --git a/dev/AppLifecycle/LaunchActivatedEventArgs.h b/dev/AppLifecycle/LaunchActivatedEventArgs.h
@@ -15,22 +15,22 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         ActivatedEventArgsBase, IInternalValueMarshalable>
     {
     public:
-        LaunchActivatedEventArgs(const std::wstring& args) : m_args(std::move(args))
+        LaunchActivatedEventArgs(const winrt::hstring args) : m_args(args)
         {
             m_kind = ActivationKind::Launch;
         }
 
         static winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri)
         {
             auto query = uri.QueryParsed();
-            auto args = std::wstring(query.GetFirstValueByName(L"Arguments"));
+            auto args = query.GetFirstValueByName(L"Arguments");
             return make<LaunchActivatedEventArgs>(args);
         }
 
         // IInternalValueMarshalable
         winrt::Windows::Foundation::Uri Serialize()
         {
-            auto uri = GenerateEncodedLaunchUri(L"App", c_launchContractId) + L"&Arguments=" + m_args;
+            auto uri = GenerateEncodedLaunchUri(L"App", c_launchContractId) + L"&Arguments=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_args.c_str());
             return winrt::Windows::Foundation::Uri(uri);
         }
 
@@ -47,6 +47,6 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         }
 
     private:
-        std::wstring m_args;
+        winrt::hstring m_args;
     };
 }
diff --git a/dev/AppLifecycle/ProtocolActivatedEventArgs.h b/dev/AppLifecycle/ProtocolActivatedEventArgs.h
@@ -16,22 +16,22 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         IInternalValueMarshalable>
     {
     public:
-        ProtocolActivatedEventArgs(const std::wstring& uri) : m_uri(winrt::Windows::Foundation::Uri(std::move(uri)))
+        ProtocolActivatedEventArgs(const winrt::hstring uri) : m_uri(winrt::Windows::Foundation::Uri(uri))
         {
             m_kind = ActivationKind::Protocol;
         }
 
         static winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri)
         {
             auto query = uri.QueryParsed();
-            auto args = std::wstring(query.GetFirstValueByName(L"Uri"));
+            auto args = query.GetFirstValueByName(L"Uri");
             return make<ProtocolActivatedEventArgs>(args);
         }
 
         // IInternalValueMarshalable
         winrt::Windows::Foundation::Uri Serialize()
         {
-            auto uri = GenerateEncodedLaunchUri(L"App", c_protocolContractId) + L"&Uri=" + m_uri.AbsoluteUri();
+            auto uri = GenerateEncodedLaunchUri(L"App", c_protocolContractId) + L"&Uri=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_uri.AbsoluteUri());
             return winrt::Windows::Foundation::Uri(uri);
         }
 
diff --git a/dev/AppLifecycle/StartupActivatedEventArgs.h b/dev/AppLifecycle/StartupActivatedEventArgs.h
@@ -15,22 +15,22 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         ActivatedEventArgsBase, IInternalValueMarshalable>
     {
     public:
-        StartupActivatedEventArgs(const std::wstring& taskId) : m_taskId(taskId)
+        StartupActivatedEventArgs(const winrt::hstring taskId) : m_taskId(taskId)
         {
             m_kind = ActivationKind::StartupTask;
         }
 
         static winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri)
         {
             auto query = uri.QueryParsed();
-            auto taskId = std::wstring(query.GetFirstValueByName(L"TaskId"));
+            auto taskId = query.GetFirstValueByName(L"TaskId");
             return make<StartupActivatedEventArgs>(taskId);
         }
 
         // IInternalValueMarshalable
         winrt::Windows::Foundation::Uri Serialize()
         {
-            auto uri = GenerateEncodedLaunchUri(L"App", c_startupTaskContractId) + L"&TaskId=" + m_taskId;
+            auto uri = GenerateEncodedLaunchUri(L"App", c_startupTaskContractId) + L"&TaskId=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_taskId.c_str());
             return winrt::Windows::Foundation::Uri(uri);
         }
 
@@ -41,7 +41,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         }
 
     private:
-        std::wstring m_taskId;
+        winrt::hstring m_taskId;
     };
 }
 

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`34`	`34`	`// Push past the '----' commandline argument prefix.`
`35`	`35`	`argsStart += 4;`
`36`	`36`
`37`		`- auto argsEnd = commandLine.find_first_of(L" ", argsStart);`
	`37`	`+ auto argsEnd = commandLine.find_first_of(L' ', argsStart);`
`38`	`38`
`39`	`39`	`// Separate the argument from any behind it on the command-line.`
`40`	`40`	`std::wstring argument;`
`@@ -53,7 +53,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`// We explicitly use find_first_of here, so that the resulting data may contain : as a valid character.`
`56`		`- auto argsDelim = argument.find_first_of(L":");`
	`56`	`+ auto argsDelim = argument.find_first_of(L':');`
`57`	`57`	`if (argsDelim == std::wstring::npos)`
`58`	`58`	`{`
`59`	`59`	`return { argument, L"" };`
`@@ -316,7 +316,6 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`316`	`316`	`{`
`317`	`317`	`ExtendedActivationKind kind = ExtendedActivationKind::Launch;`
`318`	`318`	`IInspectable data;`
`319`		`- bool foundArgs = false;`
`320`	`319`
`321`	`320`	`// For packaged, try to get platform args first.`
`322`	`321`	`if (HasIdentity())`
`@@ -325,13 +324,12 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`325`	`324`	`{`
`326`	`325`	`data = args;`
`327`	`326`	`kind = static_cast<ExtendedActivationKind>(args.Kind());`
`328`		`- foundArgs = true;`
`329`	`327`	`}`
`330`	`328`	`}`
`331`	`329`
`332`	`330`	`// Handle all Windows App SDK types next (both packaged and unpackaged).`
`333`		`- std::wstring commandLine = std::wstring(GetCommandLine());`
`334`		`- if (!foundArgs)`
	`331`	`+ std::wstring commandLine{ GetCommandLine() };`
	`332`	`+ if (data == nullptr)`
`335`	`333`	`{`
`336`	`334`	`auto [contractArgument, contractData] = ParseCommandLine(commandLine);`
`337`	`335`
`@@ -349,10 +347,10 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`349`	`347`	`}`
`350`	`348`	`}`
`351`	`349`
`352`		`- if (CompareStringOrdinal(contractArgument.c_str(), -1, c_protocolArgumentString, -1, TRUE) == CSTR_EQUAL)`
	`350`	`+ if (CompareStringOrdinal(contractArgument.c_str(), static_cast<int>(contractArgument.size()), c_protocolArgumentString, -1, TRUE) == CSTR_EQUAL)`
`353`	`351`	`{`
`354`	`352`	`kind = ExtendedActivationKind::Protocol;`
`355`		`- auto args = make<ProtocolActivatedEventArgs>(contractData);`
	`353`	`+ auto args = make<ProtocolActivatedEventArgs>(contractData.c_str());`
`356`	`354`	`data = args;`
`357`	`355`
`358`	`356`	`// Encoded launch is a protocol launch where the argument data is`
`@@ -364,17 +362,15 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`364`	`362`	`{`
`365`	`363`	`std::tie(kind, data) = GetEncodedLaunchActivatedEventArgs(args);`
`366`	`364`	`}`
`367`		`-`
`368`		`- foundArgs = true;`
`369`	`365`	`}`
`370`	`366`	`}`
`371`	`367`	`}`
`372`	`368`
`373`	`369`	`// All scenarios should just be marked as Launch.`
`374`		`- if (!foundArgs)`
	`370`	`+ if (data == nullptr)`
`375`	`371`	`{`
`376`	`372`	`kind = ExtendedActivationKind::Launch;`
`377`		`- data = make<LaunchActivatedEventArgs>(commandLine);`
	`373`	`+ data = make<LaunchActivatedEventArgs>(commandLine.c_str());`
`378`	`374`	`}`
`379`	`375`
`380`	`376`	`return make<AppActivationArguments>(kind, data);`
Original file line number	Diff line number	Diff line change
`@@ -39,12 +39,12 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`39`	`39`	`{`
`40`	`40`	`if (CompareStringOrdinal(pair.Name().c_str(), -1, c_contractIdKeyName, -1, TRUE) == CSTR_EQUAL)`
`41`	`41`	`{`
	`42`	`+ auto contractId = pair.Value().c_str();`
`42`	`43`	`for (const auto& extension : c_extensionMap)`
`43`	`44`	`{`
`44`		`- std::wstring contractId = pair.Value().c_str();`
`45`		`- if (CompareStringOrdinal(contractId.c_str(), -1, extension.contractId, -1, TRUE) == CSTR_EQUAL)`
	`45`	`+ if (CompareStringOrdinal(contractId, -1, extension.contractId, -1, TRUE) == CSTR_EQUAL)`
`46`	`46`	`{`
`47`		`- return { extension.kind, extension.factory(uri).as<winrt::Windows::Foundation::IInspectable>() };`
	`47`	`+ return { extension.kind, extension.factory(uri) };`
`48`	`48`	`}`
`49`	`49`	`}`
`50`	`50`	`}`
Original file line number	Diff line number	Diff line change
`@@ -10,13 +10,13 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`10`	`10`	`using namespace winrt::Windows::ApplicationModel::Activation;`
`11`	`11`	`using namespace winrt::Windows::Storage;`
`12`	`12`
`13`		`- constexpr PCWSTR c_fileContractId = L"Windows.File";`
	`13`	`+ constexpr inline PCWSTR c_fileContractId = L"Windows.File";`
`14`	`14`
`15`	`15`	`class FileActivatedEventArgs : public winrt::implements<FileActivatedEventArgs, IFileActivatedEventArgs, ActivatedEventArgsBase,`
`16`	`16`	`IInternalValueMarshalable>`
`17`	`17`	`{`
`18`	`18`	`public:`
`19`		`- FileActivatedEventArgs(const std::wstring verb, const std::wstring file, const bool delayVerification = false)`
	`19`	`+ FileActivatedEventArgs(const winrt::hstring verb, const winrt::hstring file, const bool delayVerification = false)`
`20`	`20`	`{`
`21`	`21`	`if (verb.empty())`
`22`	`22`	`{`
`@@ -47,16 +47,17 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`47`	`47`	`static winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri)`
`48`	`48`	`{`
`49`	`49`	`auto query = uri.QueryParsed();`
`50`		`- auto verb = std::wstring(query.GetFirstValueByName(L"Verb"));`
`51`		`- auto file = std::wstring(query.GetFirstValueByName(L"File"));`
	`50`	`+ auto verb = query.GetFirstValueByName(L"Verb");`
	`51`	`+ auto file = query.GetFirstValueByName(L"File");`
`52`	`52`	`return make<FileActivatedEventArgs>(verb, file);`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`// IInternalValueMarshalable`
`56`	`56`	`winrt::Windows::Foundation::Uri Serialize()`
`57`	`57`	`{`
`58`	`58`	`auto uri = GenerateEncodedLaunchUri(L"App", c_fileContractId);`
`59`		`- uri += L"&Verb=" + m_verb + std::wstring(L"&File=") + m_path;`
	`59`	`+ uri += L"&Verb=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_verb.c_str());`
	`60`	`+ uri += L"&File=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_path.c_str());`
`60`	`61`	`return winrt::Windows::Foundation::Uri(uri);`
`61`	`62`	`}`
`62`	`63`
`@@ -72,8 +73,8 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`72`	`73`	`}`
`73`	`74`
`74`	`75`	`private:`
`75`		`- std::wstring m_verb;`
`76`		`- std::wstring m_path;`
	`76`	`+ winrt::hstring m_verb;`
	`77`	`+ winrt::hstring m_path;`
`77`	`78`	`IVector<IStorageItem> m_files;`
`78`	`79`	`};`
`79`	`80`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,22 +15,22 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`15`	`15`	`ActivatedEventArgsBase, IInternalValueMarshalable>`
`16`	`16`	`{`
`17`	`17`	`public:`
`18`		`- LaunchActivatedEventArgs(const std::wstring& args) : m_args(std::move(args))`
	`18`	`+ LaunchActivatedEventArgs(const winrt::hstring args) : m_args(args)`
`19`	`19`	`{`
`20`	`20`	`m_kind = ActivationKind::Launch;`
`21`	`21`	`}`
`22`	`22`
`23`	`23`	`static winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri)`
`24`	`24`	`{`
`25`	`25`	`auto query = uri.QueryParsed();`
`26`		`- auto args = std::wstring(query.GetFirstValueByName(L"Arguments"));`
	`26`	`+ auto args = query.GetFirstValueByName(L"Arguments");`
`27`	`27`	`return make<LaunchActivatedEventArgs>(args);`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`// IInternalValueMarshalable`
`31`	`31`	`winrt::Windows::Foundation::Uri Serialize()`
`32`	`32`	`{`
`33`		`- auto uri = GenerateEncodedLaunchUri(L"App", c_launchContractId) + L"&Arguments=" + m_args;`
	`33`	`+ auto uri = GenerateEncodedLaunchUri(L"App", c_launchContractId) + L"&Arguments=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_args.c_str());`
`34`	`34`	`return winrt::Windows::Foundation::Uri(uri);`
`35`	`35`	`}`
`36`	`36`
`@@ -47,6 +47,6 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`private:`
`50`		`- std::wstring m_args;`
	`50`	`+ winrt::hstring m_args;`
`51`	`51`	`};`
`52`	`52`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,22 +16,22 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation`
`16`	`16`	`IInternalValueMarshalable>`
`17`	`17`	`{`
`18`	`18`	`public:`
`19`		`- ProtocolActivatedEventArgs(const std::wstring& uri) : m_uri(winrt::Windows::Foundation::Uri(std::move(uri)))`
	`19`	`+ ProtocolActivatedEventArgs(const winrt::hstring uri) : m_uri(winrt::Windows::Foundation::Uri(uri))`
`20`	`20`	`{`
`21`	`21`	`m_kind = ActivationKind::Protocol;`
`22`	`22`	`}`
`23`	`23`
`24`	`24`	`static winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri)`
`25`	`25`	`{`
`26`	`26`	`auto query = uri.QueryParsed();`
`27`		`- auto args = std::wstring(query.GetFirstValueByName(L"Uri"));`
	`27`	`+ auto args = query.GetFirstValueByName(L"Uri");`
`28`	`28`	`return make<ProtocolActivatedEventArgs>(args);`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`// IInternalValueMarshalable`
`32`	`32`	`winrt::Windows::Foundation::Uri Serialize()`
`33`	`33`	`{`
`34`		`- auto uri = GenerateEncodedLaunchUri(L"App", c_protocolContractId) + L"&Uri=" + m_uri.AbsoluteUri();`
	`34`	`+ auto uri = GenerateEncodedLaunchUri(L"App", c_protocolContractId) + L"&Uri=" + winrt::Windows::Foundation::Uri::EscapeComponent(m_uri.AbsoluteUri());`
`35`	`35`	`return winrt::Windows::Foundation::Uri(uri);`
`36`	`36`	`}`
`37`	`37`