From 00e4dc908c0ff902b1a7e4d97f50c2a3a512e76d Mon Sep 17 00:00:00 2001 From: tian-lt Date: Wed, 23 Jun 2021 22:28:47 +0800 Subject: [PATCH] resolve TODO items --- .../Common/LocalizationSettings.h | 18 +++--------------- src/CalcViewModel/Common/TraceLogger.cpp | 11 ++++++++--- src/CalcViewModel/Common/TraceLogger.h | 4 ++-- src/CalcViewModel/Common/Utils.h | 3 ++- .../Common/KeyboardShortcuManager.cs | 4 ---- .../GraphingCalculator.xaml.cs | 9 ++------- .../GraphingSettings.xaml.cs | 6 ++---- 7 files changed, 19 insertions(+), 36 deletions(-) diff --git a/src/CalcViewModel/Common/LocalizationSettings.h b/src/CalcViewModel/Common/LocalizationSettings.h index 666d631ca..0c1db6518 100644 --- a/src/CalcViewModel/Common/LocalizationSettings.h +++ b/src/CalcViewModel/Common/LocalizationSettings.h @@ -15,29 +15,17 @@ namespace CalculatorApp::ViewModel private: LocalizationSettings() // Use DecimalFormatter as it respects the locale and the user setting - // CSHARP_MIGRATION: TODO: - //: LocalizationSettings(LocalizationService::GetInstance()->GetRegionalSettingsAwareDecimalFormatter()) { - InitializeLocalizationSettings(LocalizationService::GetInstance()->GetRegionalSettingsAwareDecimalFormatter()); + Initialize(LocalizationService::GetInstance()->GetRegionalSettingsAwareDecimalFormatter()); } public: // This is only public for unit testing purposes. LocalizationSettings(Windows::Globalization::NumberFormatting::DecimalFormatter ^ formatter) { - InitializeLocalizationSettings(formatter); + Initialize(formatter); } - // A LocalizationSettings object is not copyable. - // CSHARP_MIGRATION: TODO: deleted and defaulted functions are not supported in managed/WinRT classes - //LocalizationSettings(const LocalizationSettings^) = delete; - //LocalizationSettings^ operator=(const LocalizationSettings^) = delete; - - // A LocalizationSettings object is not moveable. - // CSHARP_MIGRATION: TODO: Double check how should we hanlde move constrcutor and move assignment - //LocalizationSettings(LocalizationSettings&&) = delete; - //LocalizationSettings& operator=(LocalizationSettings&&) = delete; - // Provider of the singleton LocalizationSettings instance. static LocalizationSettings^ GetInstance() { @@ -202,7 +190,7 @@ namespace CalculatorApp::ViewModel private: - void InitializeLocalizationSettings(Windows::Globalization::NumberFormatting::DecimalFormatter ^ formatter) + void Initialize(Windows::Globalization::NumberFormatting::DecimalFormatter ^ formatter) { formatter->FractionDigits = 0; formatter->IsDecimalPointAlwaysDisplayed = false; diff --git a/src/CalcViewModel/Common/TraceLogger.cpp b/src/CalcViewModel/Common/TraceLogger.cpp index 2bc1d9721..d19f4e016 100644 --- a/src/CalcViewModel/Common/TraceLogger.cpp +++ b/src/CalcViewModel/Common/TraceLogger.cpp @@ -148,16 +148,21 @@ namespace CalculatorApp TraceLoggingCommon::GetInstance()->LogLevel2Event(StringReference(EVENT_NAME_EXCEPTION), fields); } - void TraceLogger::LogPlatformException(ViewMode mode, Platform::String ^ functionName, Platform::Exception ^ e) + void TraceLogger::LogPlatformExceptionInfo(CalculatorApp::ViewModel::Common::ViewMode mode, Platform::String ^ functionName, Platform::String^ message, int hresult) { auto fields = ref new LoggingFields(); fields->AddString(StringReference(CALC_MODE), NavCategory::GetFriendlyName(mode)); fields->AddString(StringReference(L"FunctionName"), functionName); - fields->AddString(StringReference(L"Message"), e->Message); - fields->AddInt32(StringReference(L"HRESULT"), e->HResult); + fields->AddString(StringReference(L"Message"), message); + fields->AddInt32(StringReference(L"HRESULT"), hresult); TraceLoggingCommon::GetInstance()->LogLevel2Event(StringReference(EVENT_NAME_EXCEPTION), fields); } + void TraceLogger::LogPlatformException(ViewMode mode, Platform::String ^ functionName, Platform::Exception ^ e) + { + LogPlatformExceptionInfo(mode, functionName, e->Message, e->HResult); + } + void TraceLogger::UpdateButtonUsage(NumbersAndOperatorsEnum button, ViewMode mode) { // IsProgrammerMode, IsScientificMode, IsStandardMode and None are not actual buttons, so ignore them diff --git a/src/CalcViewModel/Common/TraceLogger.h b/src/CalcViewModel/Common/TraceLogger.h index 97710ff4d..e82d36458 100644 --- a/src/CalcViewModel/Common/TraceLogger.h +++ b/src/CalcViewModel/Common/TraceLogger.h @@ -83,12 +83,12 @@ namespace CalculatorApp::ViewModel::Common void LogVariableSettingsChanged(Platform::String ^ setting); void LogGraphSettingsChanged(GraphSettingsType settingsType, Platform::String ^ settingValue); void LogGraphTheme(Platform::String ^ graphTheme); + void LogInputPasted(CalculatorApp::ViewModel::Common::ViewMode mode); + void LogPlatformExceptionInfo(CalculatorApp::ViewModel::Common::ViewMode mode, Platform::String ^ functionName, Platform::String ^ message, int hresult); internal: - // CSHARP_MIGRATION: TODO: void LogPlatformException(CalculatorApp::ViewModel::Common::ViewMode mode, Platform::String ^ functionName, Platform::Exception ^ e); void LogStandardException(CalculatorApp::ViewModel::Common::ViewMode mode, std::wstring_view functionName, _In_ const std::exception& e); - void LogInputPasted(CalculatorApp::ViewModel::Common::ViewMode mode); private: // Create an instance of TraceLogger diff --git a/src/CalcViewModel/Common/Utils.h b/src/CalcViewModel/Common/Utils.h index 53033cf3c..b15f23e2e 100644 --- a/src/CalcViewModel/Common/Utils.h +++ b/src/CalcViewModel/Common/Utils.h @@ -708,7 +708,8 @@ namespace CalculatorApp namespace ViewModel::Common { - // CSHARP_MIGRATION: TODO: Review below utils + // below utilities are intended to support interops between C# and C++/CX + // they can be removed if the entire codebase has been migrated to C# public ref class Utilities sealed { public: diff --git a/src/Calculator/Common/KeyboardShortcuManager.cs b/src/Calculator/Common/KeyboardShortcuManager.cs index 0f9213189..63a750c5b 100644 --- a/src/Calculator/Common/KeyboardShortcuManager.cs +++ b/src/Calculator/Common/KeyboardShortcuManager.cs @@ -734,8 +734,6 @@ private static void OnAcceleratorKeyActivated(CoreDispatcher dispatcher, Acceler } } - // CSHARP_MIGRATION: TODO: reinterpreted SortedDictionary> to SortedDictionary> - // double check this is equivalent before and after migration private static SortedDictionary> GetCurrentKeyDictionary(bool controlKeyPressed, bool shiftKeyPressed, bool altPressed) { int viewId = Utilities.GetWindowId(); @@ -785,7 +783,6 @@ private static SortedDictionary> GetCurrentKey } } - // CSHARP_MIGRATION: TODO: double check below design // EqualRange is a helper function to pick a range from std::multimap. private static IEnumerable EqualRange(SortedDictionary> source, TKey key) { @@ -800,7 +797,6 @@ private static IEnumerable EqualRange(SortedDictionary(SortedDictionary> dest, Tkey key, TValue value) { diff --git a/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cs b/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cs index d9ebe597a..4b47241d1 100644 --- a/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cs +++ b/src/Calculator/Views/GraphingCalculator/GraphingCalculator.xaml.cs @@ -353,13 +353,11 @@ private void OnShareClick(object sender, RoutedEventArgs e) catch (System.Runtime.InteropServices.COMException ex) { // COMException and HResult, long RPC_E_SERVERCALL_RETRYLATER is out of range of int - // LogPlatformException is internal long rpc_e_servercall_retrylater = 0x8001010A; if (ex.HResult == unchecked(rpc_e_servercall_retrylater)) { ShowShareError(); - // CSHARP_MIGRATION: TODO: - //TraceLogger.GetInstance().LogPlatformException(ViewMode.Graphing, System.Reflection.MethodBase.GetCurrentMethod().Name, ex); + CalculatorApp.ViewModel.Common.TraceLogger.GetInstance().LogPlatformExceptionInfo(ViewMode.Graphing, System.Reflection.MethodBase.GetCurrentMethod().Name, ex.Message, ex.HResult); } else { @@ -505,10 +503,7 @@ private void OnDataRequested(DataTransferManager sender, DataRequestedEventArgs catch (Exception ex) { ShowShareError(); - - // CSHARP_MIGRATION: TODO: - //TraceLogger.GetInstance().LogPlatformException(ViewMode.Graphing, - // System.Reflection.MethodBase.GetCurrentMethod().Name, ex); + CalculatorApp.ViewModel.Common.TraceLogger.GetInstance().LogPlatformExceptionInfo(ViewMode.Graphing, System.Reflection.MethodBase.GetCurrentMethod().Name, ex.Message, ex.HResult); } } diff --git a/src/Calculator/Views/GraphingCalculator/GraphingSettings.xaml.cs b/src/Calculator/Views/GraphingCalculator/GraphingSettings.xaml.cs index e056a4710..7b28dbbd5 100644 --- a/src/Calculator/Views/GraphingCalculator/GraphingSettings.xaml.cs +++ b/src/Calculator/Views/GraphingCalculator/GraphingSettings.xaml.cs @@ -110,11 +110,9 @@ private void GridSettingsTextBox_PreviewKeyDown(object sender, KeyRoutedEventArg { if (e.Key == VirtualKey.Enter) { - // CSHARP_MIGRATION: TODO: - // check if condition - if (FocusManager.TryMoveFocusAsync(FocusNavigationDirection.Next).Status != AsyncStatus.Completed) + if (FocusManager.TryMoveFocusAsync(FocusNavigationDirection.Next) == null) { - IAsyncOperation result = FocusManager.TryMoveFocusAsync(FocusNavigationDirection.Previous); + _ = FocusManager.TryMoveFocusAsync(FocusNavigationDirection.Previous); } e.Handled = true; }