Skip to content

Add an option to expose the allowMidWordMatch as a setting#152292

Merged
jrieken merged 24 commits intomicrosoft:mainfrom
etriebe:etriebe/add_firstmatchcanbeweak_option
Aug 22, 2022
Merged

Add an option to expose the allowMidWordMatch as a setting#152292
jrieken merged 24 commits intomicrosoft:mainfrom
etriebe:etriebe/add_firstmatchcanbeweak_option

Conversation

@etriebe
Copy link

@etriebe etriebe commented Jun 16, 2022

This PR fixes #53715

This won't change the default experience but will give people an option to change the fuzzy search algorithm that is used.

How to test:

  1. Create a new C# test.cs file
  2. install omnisharp
  3. Populate the file with this
using System;
using System.Collections.Generic;

public class FuzzyTest
{

    public FuzzyTest()
    {
        // Type here
    }
}

public class FBUserSession
{
    string test;
    string test2;
}
  1. In constructor, type FBUserSession, UserSession and Session. The first and last will work with the setting off.
  2. Turn the new setting on
  3. Redo test for step 4 and see that all those words typed will return the FBUserSession object that the user wants

@ghost
Copy link

ghost commented Jun 16, 2022 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

jrieken
jrieken requested changes Jun 20, 2022
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

Thanks so far. This is already looking quite good. I left a few nit comments for seemingly un-needed changes (please PR minimally). We also need to come up with a good name, IMO firstMatchCanBeWeak is too technical to expose it as user-facing setting

@etriebe
Copy link
Author

etriebe commented Jun 20, 2022

Thanks so far. This is already looking quite good. I left a few nit comments for seemingly un-needed changes (please PR minimally). We also need to come up with a good name, IMO firstMatchCanBeWeak is too technical to expose it as user-facing setting

@jrieken what about allowMidWordMatch? Because I think this is what this current change will do. Or we change logic to have the inverse, which is something like, forceFirstTypedCharacterToBeStartOfWord or forceMatchOnStartOfWord. I might prefer the second.

@etriebe
Copy link
Author

etriebe commented Jun 21, 2022

@jrieken all issues should be resolved besides the name choice which I'd love your feedback on before making a choice.

Also, as an aside, what formatter do you use for format document to work to avoid hygiene issues? I used prettier and then the whole doc changed significantly.

@jrieken jrieken added this to the July 2022 milestone Jun 27, 2022
@etriebe etriebe changed the title Add an option to expose the firstMatchCanBeWeak as a setting Add an option to expose the allowMidWordMatch as a setting Jun 27, 2022
@etriebe etriebe requested a review from jrieken June 27, 2022 18:08
@etriebe
Copy link
Author

etriebe commented Jun 30, 2022

Any chance you can take a look? @jrieken

@jrieken jrieken modified the milestones: July 2022, August 2022 Jul 21, 2022
@jrieken
Copy link
Member

jrieken commented Jul 21, 2022

Sorry - I have been swapped recently and I am now heading out to a longer vacation. My plan is to finally pick this up in August. Thanks for being patient.

@etriebe
Copy link
Author

etriebe commented Jul 21, 2022

Sorry - I have been swapped recently and I am now heading out to a longer vacation. My plan is to finally pick this up in August. Thanks for being patient.

All good! Thanks for the heads up! We're going to work on a fix internally for now and will revert that once this lands here. 😀

@etriebe
Copy link
Author

etriebe commented Aug 16, 2022

Any chance you'll be able to take a look for August? @jrieken

jrieken
jrieken reviewed Aug 19, 2022
View reviewed changes
@etriebe etriebe requested a review from jrieken August 19, 2022 16:40
jrieken
jrieken approved these changes Aug 22, 2022
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

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

Thanks. I have pushed a change with the new name for this (matchOnWordStartOnly) and this is now ready to be merged

@jrieken jrieken merged commit eab7774 into microsoft:main Aug 22, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jrieken jrieken jrieken approved these changes

@sandy081 sandy081 sandy081 approved these changes

Assignees

@jrieken jrieken

Labels

None yet

Projects

None yet

Milestone

August 2022

Development

Successfully merging this pull request may close these issues.

suggestion score - allow first character to be a weak match

3 participants

@etriebe @jrieken @sandy081