ronantakizawa/codereview-bench huggingface dataset:
Repository: microsoft/PowerToys
File: src/modules/fancyzones/editor/FancyZonesEditor/Models/DefaultLayoutsModel.cs (C#)

Code before:
                    Set(MainWindowSettingsModel.TemplateModels[(int)LayoutType.Rows], type);
                    break;
            }
        }

        public void Reset(string uuid)
        {
            if (Layouts[(int)MonitorConfigurationType.Horizontal].Uuid == uuid)
            {
                Set(MainWindowSettingsModel.TemplateModels[(int)LayoutType.PriorityGrid], MonitorConfigurationType.Horizontal);
            }

            if (Layouts[(int)MonitorConfigurationType.Vertical].Uuid == uuid)
            {
                Set(MainWindowSettingsModel.TemplateModels[(int)LayoutType.Rows], MonitorConfigurationType.Vertical);
            }
        }

        public void Set(LayoutModel layout, MonitorConfigurationType type)
        {
            if (Layouts.Count <= (int)type)
            {
                Layouts[(int)type] = layout;
            }
            else
            {
                Layouts[(int)type] = layout;
            }

            FirePropertyChanged();
        }

        public void Restore(Dictionary<int, LayoutModel> layouts)
        {
            for (int i = 0; i < Count; i++)
            {
                Set(layouts[i], (MonitorConfigurationType)i);
            }
        }

        protected virtual void FirePropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}


Review comment (suggestion):
As both `if` and `else` have the same body, we can leave just `Layouts[type] = layout;`

Code after:
                    Set(MainWindowSettingsModel.TemplateModels[(int)LayoutType.Rows], type);
                    break;
            }
        }

        public void Reset(string uuid)
        {
            if (Layouts[MonitorConfigurationType.Horizontal].Uuid == uuid)
            {
                Set(MainWindowSettingsModel.TemplateModels[(int)LayoutType.PriorityGrid], MonitorConfigurationType.Horizontal);
            }

            if (Layouts[MonitorConfigurationType.Vertical].Uuid == uuid)
            {
                Set(MainWindowSettingsModel.TemplateModels[(int)LayoutType.Rows], MonitorConfigurationType.Vertical);
            }
        }

        public void Set(LayoutModel layout, MonitorConfigurationType type)
        {
            Layouts[type] = layout;

            FirePropertyChanged();
        }

        public void Restore(Dictionary<MonitorConfigurationType, LayoutModel> layouts)
        {
            foreach (var monitorConfigurationType in layouts.Keys)
            {
                Set(layouts[monitorConfigurationType], monitorConfigurationType);
            }
        }

        protected virtual void FirePropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }
    }
}
Repository: microsoft/PowerToys
File: src/modules/launcher/PowerLauncher/Storage/QueryHistory.cs (C#)

Code before:
            }

            if (Items.Count > 0 && Items.Last().Query == query)
            {
                Items.Last().ExecutedDateTime = DateTime.Now;
            }
            else
            {
                Items.Add(new HistoryItem
                {
                    Query = query,
                    ExecutedDateTime = DateTime.Now,
                });
            }
        }

        public void Update()
        {
            foreach (var item in Items)
            {
                if (item.ExecutedDateTime == DateTime.MinValue)
                {
                    item.ExecutedDateTime = DateTime.Now;
                }

                if (string.IsNullOrEmpty(item.Query))
                {
                    item.Query = "Default Query";
                }
            }
        }
    }
}


Review comment (suggestion):
I'm not sure if this makes sense 🤔 I'm fine with overwriting execution time, but if Query is malformed and we couldn't load it properly, overwriting it with "Default query" doesn't make it usable in the future. I'd suggest if Query is null or empty, just remove that history entry

Code after:
            }

            if (Items.Count > 0 && Items.Last().Query == query)
            {
                Items.Last().ExecutedDateTime = DateTime.Now;
            }
            else
            {
                Items.Add(new HistoryItem
                {
                    Query = query,
                    ExecutedDateTime = DateTime.Now,
                });
            }
        }

        public void Update()
        {
            for (int i = Items.Count - 1; i >= 0; i--)
            {
                if (string.IsNullOrEmpty(Items[i].Query))
                {
                    Items.RemoveAt(i);
                }
                else
                {
                    if (Items[i].ExecutedDateTime == DateTime.MinValue)
                    {
                        Items[i].ExecutedDateTime = DateTime.Now;
                    }
                }
            }
        }
    }
}
Repository: microsoft/PowerToys
File: src/modules/imageresizer/dll/ContextMenuHandler.cpp (C++)

Code before:
// ContextMenuHandler.cpp : Implementation of CContextMenuHandler

#include "pch.h"
#include "ContextMenuHandler.h"

#include <Settings.h>
#include <trace.h>

#include <common/themes/icon_helpers.h>
#include <common/utils/process_path.h>
#include <common/utils/resources.h>
#include <common/utils/HDropIterator.h>
#include <common/utils/package.h>

extern HINSTANCE g_hInst_imageResizer;

CContextMenuHandler::CContextMenuHandler()
{
    m_pidlFolder = NULL;
    m_pdtobj = NULL;
    context_menu_caption = GET_RESOURCE_STRING_FALLBACK(IDS_IMAGERESIZER_CONTEXT_MENU_ENTRY, L"Resize with Image Resizer");
    context_menu_caption_here = GET_RESOURCE_STRING_FALLBACK(IDS_IMAGERESIZER_CONTEXT_MENU_ENTRY_HERE, L"Resize with Image Resizer");
}

CContextMenuHandler::~CContextMenuHandler()
{
    Uninitialize();
}

void CContextMenuHandler::Uninitialize()
{
    CoTaskMemFree((LPVOID)m_pidlFolder);
    m_pidlFolder = NULL;

    if (m_pdtobj)
    {
        m_pdtobj->Release();
        m_pdtobj = NULL;
    }
}

HRESULT CContextMenuHandler::Initialize(_In_opt_ PCIDLIST_ABSOLUTE pidlFolder, _In_opt_ IDataObject* pdtobj, _In_opt_ HKEY /*hkeyProgID*/)
{
    Uninitialize();

    if (!CSettingsInstance().GetEnabled())
    {

Review comment (suggestion):
```suggestion
    context_menu_caption_here = GET_RESOURCE_STRING_FALLBACK(IDS_IMAGERESIZER_CONTEXT_MENU_ENTRY_HERE, L"Resize with Image Resizer here");
```

Code after:
// ContextMenuHandler.cpp : Implementation of CContextMenuHandler

#include "pch.h"
#include "ContextMenuHandler.h"

#include <Settings.h>
#include <trace.h>

#include <common/themes/icon_helpers.h>
#include <common/utils/process_path.h>
#include <common/utils/resources.h>
#include <common/utils/HDropIterator.h>
#include <common/utils/package.h>

extern HINSTANCE g_hInst_imageResizer;

CContextMenuHandler::CContextMenuHandler()
{
    m_pidlFolder = NULL;
    m_pdtobj = NULL;
    context_menu_caption = GET_RESOURCE_STRING_FALLBACK(IDS_IMAGERESIZER_CONTEXT_MENU_ENTRY, L"Resize with Image Resizer");
    context_menu_caption_here = GET_RESOURCE_STRING_FALLBACK(IDS_IMAGERESIZER_CONTEXT_MENU_ENTRY_HERE, L"Resize with Image Resizer here");
}

CContextMenuHandler::~CContextMenuHandler()
{
    Uninitialize();
}

void CContextMenuHandler::Uninitialize()
{
    CoTaskMemFree((LPVOID)m_pidlFolder);
    m_pidlFolder = NULL;

    if (m_pdtobj)
    {
        m_pdtobj->Release();
        m_pdtobj = NULL;
    }
}

HRESULT CContextMenuHandler::Initialize(_In_opt_ PCIDLIST_ABSOLUTE pidlFolder, _In_opt_ IDataObject* pdtobj, _In_opt_ HKEY /*hkeyProgID*/)
{
    Uninitialize();

    if (!CSettingsInstance().GetEnabled())
    {
Repository: microsoft/PowerToys
File: src/modules/PowerOCR/PowerOCR/Helpers/WPFExtensionMethods.cs (C#)

Code before:
            IntPtr hmonitor = OSInterop.MonitorFromWindow(new HandleRef(null, helper.EnsureHandle()), 2);
            OSInterop.MONITORINFOEX info = new OSInterop.MONITORINFOEX();
            OSInterop.GetMonitorInfo(new HandleRef(null, hmonitor), info);
            r = new Int32Rect(info.RcMonitor.Left, info.RcMonitor.Top, info.RcMonitor.Width, info.RcMonitor.Height);
        }

        return new Point(r.X, r.Y);
    }

    public static DpiScale GetDpi(this System.Windows.Forms.Screen screen)
    {
        var point = new System.Drawing.Point(screen.Bounds.Left + 1, screen.Bounds.Top + 1);
        var mon = MonitorFromPoint(point, 2/*MONITOR_DEFAULTTONEAREST*/);
        GetDpiForMonitor(mon, DpiType.Effective, out uint dpiX, out uint dpiY);
        return new DpiScale(dpiX / 96.0, dpiY / 96.0);
    }

    // https://msdn.microsoft.com/en-us/library/windows/desktop/dd145062(v=vs.85).aspx
    [DllImport("User32.dll")]
    private static extern IntPtr MonitorFromPoint([In] System.Drawing.Point pt, [In] uint dwFlags);

    // https://msdn.microsoft.com/en-us/library/windows/desktop/dn280510(v=vs.85).aspx
    [DllImport("Shcore.dll")]
    private static extern IntPtr GetDpiForMonitor([In] IntPtr hmonitor, [In] DpiType dpiType, [Out] out uint dpiX, [Out] out uint dpiY);

    // https://msdn.microsoft.com/en-us/library/windows/desktop/dn280511(v=vs.85).aspx
    public enum DpiType
    {
        Effective = 0,
        Angular = 1,
        Raw = 2,
    }
}


Review comment (suggestion):
Nit
```suggestion
    // https://msdn.microsoft.com/library/windows/desktop/dn280511(v=vs.85).aspx
```


Code after:
            IntPtr hmonitor = OSInterop.MonitorFromWindow(new HandleRef(null, helper.EnsureHandle()), 2);
            OSInterop.MONITORINFOEX info = new OSInterop.MONITORINFOEX();
            OSInterop.GetMonitorInfo(new HandleRef(null, hmonitor), info);
            r = new Int32Rect(info.RcMonitor.Left, info.RcMonitor.Top, info.RcMonitor.Width, info.RcMonitor.Height);
        }

        return new Point(r.X, r.Y);
    }

    public static DpiScale GetDpi(this System.Windows.Forms.Screen screen)
    {
        var point = new System.Drawing.Point(screen.Bounds.Left + 1, screen.Bounds.Top + 1);
        var mon = MonitorFromPoint(point, 2/*MONITOR_DEFAULTTONEAREST*/);
        GetDpiForMonitor(mon, DpiType.Effective, out uint dpiX, out uint dpiY);
        return new DpiScale(dpiX / 96.0, dpiY / 96.0);
    }

    // https://msdn.microsoft.com/library/windows/desktop/dd145062(v=vs.85).aspx
    [DllImport("User32.dll")]
    private static extern IntPtr MonitorFromPoint([In] System.Drawing.Point pt, [In] uint dwFlags);

    // https://msdn.microsoft.com/library/windows/desktop/dn280510(v=vs.85).aspx
    [DllImport("Shcore.dll")]
    private static extern IntPtr GetDpiForMonitor([In] IntPtr hmonitor, [In] DpiType dpiType, [Out] out uint dpiX, [Out] out uint dpiY);

    // https://msdn.microsoft.com/library/windows/desktop/dn280511(v=vs.85).aspx
    public enum DpiType
    {
        Effective = 0,
        Angular = 1,
        Raw = 2,
    }
}
Repository: microsoft/PowerToys
File: src/modules/launcher/Plugins/Microsoft.Plugin.WindowWalker/Components/WindowProcess.cs (C#)

Code before:
    internal class WindowProcess
    {
        /// <summary>
        /// Maximum size of a file name
        /// </summary>
        private const int MaximumFileNameLength = 1000;

        /// <summary>
        /// An indicator if the window belongs to an 'Universal Windows Platform (UWP)' process
        /// </summary>
        private readonly bool _isUwpApp;

        /// <summary>
        /// Gets the id of the process
        /// </summary>
        internal uint ProcessID
        {
            get; private set;
        }

        /// <summary>
        /// Gets a value indicating whether the process is responding or not
        /// </summary>
        internal bool IsResponding
        {
            get { return Process.GetProcessById((int)ProcessID).Responding; }
        }

        /// <summary>
        /// Gets the id of the thread
        /// </summary>
        internal uint ThreadID
        {
            get; private set;
        }

        /// <summary>
        /// Gets the name of the process
        /// </summary>
        internal string Name
        {
            get; private set;
        }

        /// <summary>
        /// Gets a value indicating whether the window belongs to an 'Universal Windows Platform (UWP)' process
        /// </summary>
        internal bool IsUwpApp
        {
            get { return _isUwpApp; }
        }

Review comment (suggestion):
According to the docs, this might cause an Exception, which we should catch: https://learn.microsoft.com/en-us/dotnet/api/system.diagnostics.process.responding?view=net-8.0
I guess by default we should return true if we get an exception.

Code after:
    internal class WindowProcess
    {
        /// <summary>
        /// Maximum size of a file name
        /// </summary>
        private const int MaximumFileNameLength = 1000;

        /// <summary>
        /// An indicator if the window belongs to an 'Universal Windows Platform (UWP)' process
        /// </summary>
        private readonly bool _isUwpApp;

        /// <summary>
        /// Gets the id of the process
        /// </summary>
        internal uint ProcessID
        {
            get; private set;
        }

        /// <summary>
        /// Gets a value indicating whether the process is responding or not
        /// </summary>
        internal bool IsResponding
        {
            get
            {
                try
                {
                    return Process.GetProcessById((int)ProcessID).Responding;
                }
                catch (InvalidOperationException)
                {
                    // Thrown when process not exist.
                    return true;
                }
                catch (NotSupportedException)
                {
                    // Thrown when process is not running locally.
                    return true;
                }
            }
        }

        /// <summary>
        /// Gets the id of the thread
        /// </summary>
        internal uint ThreadID
        {
            get; private set;
        }
Repository: microsoft/PowerToys
File: src/modules/AdvancedPaste/AdvancedPaste/Helpers/AICompletionsHelper.cs (C#)

Code before:
            {
                rawAIResponse = this.GetAICompletion(systemInstructions, userMessage);
                aiResponse = rawAIResponse.Value.Choices[0].Text;
            }
            catch (Azure.RequestFailedException error)
            {
                Logger.LogError("GetAICompletion failed", error);
                PowerToysTelemetry.Log.WriteEvent(new Telemetry.AdvancedPasteGenerateCustomErrorEvent(error.Message));
                apiRequestStatus = error.Status;
            }
            catch (Exception error)
            {
                Logger.LogError("GetAICompletion failed", error);
                PowerToysTelemetry.Log.WriteEvent(new Telemetry.AdvancedPasteGenerateCustomErrorEvent(error.Message));
                apiRequestStatus = -1;
            }

            int promptTokens = -1;
            int completionTokens = -1;
            if (rawAIResponse != null)
            {
                promptTokens = rawAIResponse.Value.Usage.PromptTokens;
                completionTokens = rawAIResponse.Value.Usage.CompletionTokens;
            }

            PowerToysTelemetry.Log.WriteEvent(new Telemetry.AdvancedPasteGenerateCustomFormatEvent(promptTokens, completionTokens, "gpt-3.5-turbo-instruct"));

            return new AICompletionsResponse(aiResponse, apiRequestStatus);
        }
    }
}


Review comment (suggestion):
uplevel "gpt-3.5-turbo-instruct" into shared var.

it is used at a min on `DeploymentName = "gpt-3.5-turbo-instruct",` higher up in code

Code after:
            int apiRequestStatus = (int)HttpStatusCode.OK;
            try
            {
                rawAIResponse = this.GetAICompletion(systemInstructions, userMessage);
                aiResponse = rawAIResponse.Value.Choices[0].Text;

                int promptTokens = rawAIResponse.Value.Usage.PromptTokens;
                int completionTokens = rawAIResponse.Value.Usage.CompletionTokens;
                PowerToysTelemetry.Log.WriteEvent(new Telemetry.AdvancedPasteGenerateCustomFormatEvent(promptTokens, completionTokens, _modelName));
            }
            catch (Azure.RequestFailedException error)
            {
                Logger.LogError("GetAICompletion failed", error);
                PowerToysTelemetry.Log.WriteEvent(new Telemetry.AdvancedPasteGenerateCustomErrorEvent(error.Message));
                apiRequestStatus = error.Status;
            }
            catch (Exception error)
            {
                Logger.LogError("GetAICompletion failed", error);
                PowerToysTelemetry.Log.WriteEvent(new Telemetry.AdvancedPasteGenerateCustomErrorEvent(error.Message));
                apiRequestStatus = -1;
            }

            return new AICompletionsResponse(aiResponse, apiRequestStatus);
        }
    }
}
Repository: microsoft/PowerToys
File: src/modules/Workspaces/WorkspacesLauncherUI/Models/AppLaunching.cs (C#)

Code before:
                            _icon = Icon.ExtractAssociatedIcon(AppPath);
                        }
                    }
                    catch (Exception)
                    {
                        Logger.LogWarning($"Icon not found on app path: {AppPath}. Using default icon");
                        IsNotFound = true;
                        _icon = new Icon(@"images\DefaultIcon.ico");
                    }
                }

                return _icon;
            }
        }

        public string Name { get; set; }

        public string LaunchState { get; set; }

        public string StateImageSource
        {
            get => LaunchState switch
            {
                "launched" => "../images/checkmark.png",
                "failed" => "../images/failed.png",
                _ => "../images/failed.gif",
            };
        }

        private bool _isNotFound;

        [JsonIgnore]
        public bool IsNotFound
        {
            get
            {
                return _isNotFound;
            }

            set
            {
                if (_isNotFound != value)
                {
                    _isNotFound = value;
                    OnPropertyChanged(new PropertyChangedEventArgs(nameof(IsNotFound)));
                }
            }
        }

        public Uri GetAppLogoByPackageFamilyName()
        {

Review comment (bug):
```suggestion
                _ => "../images/failed.png",
```

Code after:
                            _icon = Icon.ExtractAssociatedIcon(AppPath);
                        }
                    }
                    catch (Exception)
                    {
                        Logger.LogWarning($"Icon not found on app path: {AppPath}. Using default icon");
                        IsNotFound = true;
                        _icon = new Icon(@"images\DefaultIcon.ico");
                    }
                }

                return _icon;
            }
        }

        public string Name { get; set; }

        public string LaunchState { get; set; }

        public string StateImageSource
        {
            get => LaunchState switch
            {
                "launched" => "../images/checkmark.png",
                "failed" => "../images/failed.png",
                _ => "../images/failed.png",
            };
        }

        private bool _isNotFound;

        [JsonIgnore]
        public bool IsNotFound
        {
            get
            {
                return _isNotFound;
            }

            set
            {
                if (_isNotFound != value)
                {
                    _isNotFound = value;
                    OnPropertyChanged(new PropertyChangedEventArgs(nameof(IsNotFound)));
                }
            }
        }

        public Uri GetAppLogoByPackageFamilyName()
        {
Repository: microsoft/PowerToys
File: src/modules/keyboardmanager/common/Helpers.cpp (C++)

Code before:
        case VK_END:
            numpad_originated = !extended;
            break;
        case VK_RETURN:
        case VK_DIVIDE:
            numpad_originated = extended;
            break;
        }

        if (numpad_originated)
            return key | GetNumpadOriginEncodingBit();
        else
            return key;
    }

    DWORD ClearKeyNumpadOrigin(const DWORD key)
    {
        return (key & ~GetNumpadOriginEncodingBit());
    }

    bool IsNumpadOriginated(const DWORD key)
    {
        return !!(key & GetNumpadOriginEncodingBit());
    }

    bool IsNumpadKey(const DWORD vkCode)
    {
        switch (vkCode)
        {
        case VK_NUMPAD0:
        case VK_NUMPAD1:
        case VK_NUMPAD2:
        case VK_NUMPAD3:
        case VK_NUMPAD4:
        case VK_NUMPAD5:
        case VK_NUMPAD6:
        case VK_NUMPAD7:
        case VK_NUMPAD8:
        case VK_NUMPAD9:
        case VK_DECIMAL:
            return true;
        }
        return false;
    }

    DWORD GetNumpadOriginEncodingBit()
    {
        // Intentionally do not mimic KF_EXTENDED to avoid confusion, because it's not the same thing
        // See EncodeKeyNumpadOrigin.
        return 1ull << 31;
    }

Review comment (suggestion):
```suggestion
    // Check if it's one of the numpad keys that the shift key can affect, so we can introduce a workaround when they are mapped to shift.
    bool IsNumpadKeyThatIsAffectedByShift(const DWORD vkCode)
```

Code after:
        case VK_END:
            numpad_originated = !extended;
            break;
        case VK_RETURN:
        case VK_DIVIDE:
            numpad_originated = extended;
            break;
        }

        if (numpad_originated)
            return key | GetNumpadOriginEncodingBit();
        else
            return key;
    }

    DWORD ClearKeyNumpadOrigin(const DWORD key)
    {
        return (key & ~GetNumpadOriginEncodingBit());
    }

    bool IsNumpadOriginated(const DWORD key)
    {
        return !!(key & GetNumpadOriginEncodingBit());
    }

    // Check if it's one of the numpad keys that the shift key can affect, so we can introduce a workaround when they are mapped to shift.
    bool IsNumpadKeyThatIsAffectedByShift(const DWORD vkCode)
    {
        switch (vkCode)
        {
        case VK_NUMPAD0:
        case VK_NUMPAD1:
        case VK_NUMPAD2:
        case VK_NUMPAD3:
        case VK_NUMPAD4:
        case VK_NUMPAD5:
        case VK_NUMPAD6:
        case VK_NUMPAD7:
        case VK_NUMPAD8:
        case VK_NUMPAD9:
        case VK_DECIMAL:
            return true;
        }
        return false;
    }

    DWORD GetNumpadOriginEncodingBit()
    {
        // Intentionally do not mimic KF_EXTENDED to avoid confusion, because it's not the same thing
        // See EncodeKeyNumpadOrigin.
        return 1ull << 31;