Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In most cases, remove the camel case matching for case sensitive matches #70324

Merged
merged 2 commits into from
Oct 12, 2023

Conversation

ToddGrun
Copy link
Contributor

There is no need to do this unless the pattern is successfully matched case insensitively. Prism has marked several PatternMatcher calls under TryCamelCaseMatch when looking at CPU stacks for the completion scenario.

…hes. There is no need to do this unless the pattern is successfully matched case insensitively. Prism has marked several PatternMatcher calls under TryCamelCaseMatch when looking at CPU stacks for the completion scenario.
@ToddGrun ToddGrun requested a review from a team as a code owner October 10, 2023 20:58
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Oct 10, 2023
@ToddGrun ToddGrun requested a review from genlu October 10, 2023 20:59
@ToddGrun
Copy link
Contributor Author

ToddGrun commented Oct 10, 2023

Prism bug: AB#1899339

@@ -446,20 +445,21 @@ private bool PartStartsWith(string candidate, TextSpan candidatePart, string pat
// i.e. CoFiPro would match CodeFixProvider, but CofiPro would not.
if (patternChunk.PatternHumps.Count > 0)
{
var camelCaseKind = TryUpperCaseCamelCaseMatch(candidate, candidateHumps, patternChunk, CompareOptions.None, out var matchedSpans);
if (camelCaseKind.HasValue)
// No need to do the case sensitive check unless the case insensitive check is successful
Copy link
Member

Choose a reason for hiding this comment

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

as per our offline convo, it might be nice to have a little more verbosity explaining why this is good.

namely that the expected case is to test the pattern matcher against thousands of items, with only a handful matching. As such, the existing approach takes 2x checks against all those thousands of items. Whereas the new approach does (1x + handful) checks.

@ToddGrun ToddGrun merged commit f0c5469 into dotnet:main Oct 12, 2023
24 checks passed
@ghost ghost added this to the Next milestone Oct 12, 2023
@jjonescz jjonescz modified the milestones: Next, 17.9 P1 Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants