Skip to content

Migrate files from Wox to PowerLauncher#5014

Merged
dsrivastavv merged 16 commits into
microsoft:masterfrom
dsrivastavv:somil55/RemoveWox
Jul 20, 2020
Merged

Migrate files from Wox to PowerLauncher#5014
dsrivastavv merged 16 commits into
microsoft:masterfrom
dsrivastavv:somil55/RemoveWox

Conversation

@dsrivastavv

@dsrivastavv dsrivastavv commented Jul 15, 2020

Copy link
Copy Markdown
Contributor

Summary of the Pull Request

This PR removes Wox project and migrates all class into PowerLauncher project.

PR Checklist

Detailed Description of the Pull Request / Additional comments

This PR makes the following changes :

  1. Move all files from Wox to PowerLauncher project.
  2. Fix all static analysis errors in migrated files.
  3. Delete Wox project.
  4. Updated MSI.

Following static analysis errors have been suppressed in MainViewModel.cs :

#pragma warning disable CA1308 // Normalize strings to uppercase
            var query = QueryText.ToLower(CultureInfo.InvariantCulture).Trim();
#pragma warning restore CA1308 // Normalize strings to uppercase

We currently convert all strings to lowercase across all plugins. This would require updating all plugins and projects to use ToUpper function for string normalization. It would be better to make this change as part of seperate PR.

#pragma warning disable CA1031 // Do not catch general exception types
            catch (Exception)
#pragma warning restore CA1031 // Do not catch general exception types

This error asks to use more specific exception type in catch block. Since this is an interop code, it is not very clear which specific exception should be used.

Validation Steps Performed

Manually verified that all plugins function correctly and MSI is built without any errors.

@dsrivastavv dsrivastavv added the Product-PowerToys Run Improved app launch PT Run (Win+R) Window label Jul 15, 2020
@dsrivastavv dsrivastavv changed the title Somil55/remove wox Remove Wox project Jul 15, 2020
@dsrivastavv dsrivastavv changed the title Remove Wox project Migrate files from Wox to PowerLauncher Jul 15, 2020
@dsrivastavv
dsrivastavv requested a review from a team July 15, 2020 23:01
@dsrivastavv
dsrivastavv marked this pull request as ready for review July 15, 2020 23:01
{
var key = result.ToString();
if (records.TryGetValue(key, out int value))
if(result != null)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@somil55 In what cases are we expecting result to be null? If we don't know this should probably be a nullargexception, otherwise we should add a comment on what cases it's expected to be null and why we guard against it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ryanbodrug-microsoft All calls to Add are preceded by null check. So in our codebase, it would ever be null. I will update this to throw nullargexception.

Comment thread src/modules/launcher/PowerLauncher/ViewModel/MainViewModel.cs Outdated
Comment thread src/modules/launcher/PowerLauncher/ViewModel/ResultsViewModel.cs Outdated
Comment thread src/modules/launcher/PowerLauncher/ViewModel/RelayCommand.cs
Comment thread src/modules/launcher/PowerLauncher/Storage/QueryHistory.cs
Comment on lines +153 to +178
[DllImport("user32.dll")]
internal static extern uint SendInput(uint nInputs, INPUT[] pInputs, int cbSize);

[DllImport("user32.dll", SetLastError = true)]
internal static extern int GetWindowLong(IntPtr hWnd, int nIndex);

[DllImport("user32.dll")]
internal static extern int SetWindowLong(IntPtr hWnd, int nIndex, int dwNewLong);

[DllImport("user32.dll")]
internal static extern IntPtr GetForegroundWindow();

[DllImport("user32.dll")]
internal static extern IntPtr GetDesktopWindow();

[DllImport("user32.dll")]
internal static extern IntPtr GetShellWindow();

[DllImport("user32.dll", SetLastError = true)]
internal static extern int GetWindowRect(IntPtr hwnd, out RECT rc);

[DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
internal static extern int GetClassName(IntPtr hWnd, StringBuilder lpClassName, int nMaxCount);

[DllImport("user32.DLL", CharSet = CharSet.Unicode)]
internal static extern IntPtr FindWindowEx(IntPtr hwndParent, IntPtr hwndChildAfter, string lpszClass, string lpszWindow);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@somil55 Can you help me understand why these need to be added? Where they moved from somewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They were moved from WindowsInteropHelper.cs. This was to fix CA1060 error, which requires all native methods in a namespace to be moved to a static nativemethod class.

@ryanbodrug-microsoft ryanbodrug-microsoft left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dsrivastavv
dsrivastavv merged commit c85cd4a into microsoft:master Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Product-PowerToys Run Improved app launch PT Run (Win+R) Window

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants