Skip to content

Conversation

@jjw24
Copy link
Member

@jjw24 jjw24 commented Jul 25, 2021

Resolves the issue where the Windows Search service required for Windows Index Search in Explorer plugin is turned off by the user intentionally and currently throwing the error report window on every character typed.

To replicate this issue, you need to stop Windows Search service, disable it so it wont be activated by other services, and then type in the search window to trigger Explorer 's Windows Index Search.

Solution:

  • Add exception handling, and then output a warning window to let user know.
  • Add option for user to remove the message, as well as an alternative to install Everything plugin.
    Users who intentionally stopped the service usually are inclined to use Everything instead.

image

image

image

Close #581

@jjw24 jjw24 added the bug Something isn't working label Jul 25, 2021
@jjw24 jjw24 added this to the 1.9.0 milestone Jul 25, 2021
@jjw24 jjw24 self-assigned this Jul 25, 2021
@jjw24 jjw24 enabled auto-merge July 25, 2021 10:46

internal async static Task<List<Result>> WindowsIndexSearchAsync(string searchString, string connectionString,
internal async static Task<List<Result>> WindowsIndexSearchAsync(string searchString,
Func<CSearchQueryHelper> queryHelper,
Copy link
Member

Choose a reason for hiding this comment

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

why not simply accept a CSearchQueryHelper here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well not much of difference at this stage to pass in the actual object or the function. Just thought to keep it consistent with other parameters, and in the future if we do need to test this function we don't need to mock up the object or change it to a function parameter

Copy link
Member

Choose a reason for hiding this comment

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

Hmm even with the delegate seems that we still need to mock the object for testing🧐.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, with Explorer's approach you can test each individual function for their behaviour

Copy link
Member

Choose a reason for hiding this comment

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

I mean we still need to mock a CSearchQueryHelper to test this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Creating a connection string calls CSearchManager, that's why I had to change it

Copy link
Member

Choose a reason for hiding this comment

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

Sorry i didn't notice the message.
I think it would be better to keep the dependency cleaner. This method only requires a connection string, so probably creating it outside and pass that in would be better.
I don't think the c search manager belongs to this method, so why not create it outside the method?

Too much delegate may make the code a bit hard to understand.

Copy link
Member Author

Choose a reason for hiding this comment

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

CSearchManager is part of this method call chain, other alternative is to pass in the query helper object, do this instead?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I mean so. I just not quite understand the need to pass a delegate here.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@jjw24 jjw24 requested a review from taooceros July 27, 2021 08:04
@taooceros taooceros disabled auto-merge July 27, 2021 08:15
@taooceros taooceros enabled auto-merge July 27, 2021 08:15
@taooceros taooceros merged commit 7a355d4 into dev Jul 27, 2021
@taooceros taooceros deleted the fix_indexing_notrunning branch July 27, 2021 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Search Crashing due to Windows Index Search service not running

3 participants