Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

add fuzzy code hint searching #7441

Merged
merged 8 commits into from
Apr 24, 2014
Merged

add fuzzy code hint searching #7441

merged 8 commits into from
Apr 24, 2014

Conversation

JeffryBooher
Copy link
Contributor

Adds fuzzy code hints to css editing:

Known issues:

shape-inside: circle( <-- presents shapes again. This exists in master because we don't set the context o

var lastContext;
var lastContext,
matcher = null, // string matcher for hints
prefs = PreferencesManager.getExtensionPrefs("CSSCodeHints");
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need a pref for this? As long as the behavior we offer is not a cause for complaint (or something people reasonably disagree on), then I don't think a knob is needed.

@dangoor
Copy link
Contributor

dangoor commented Apr 14, 2014

Initial review done.

@JeffryBooher
Copy link
Contributor Author

@dangoor All feedback addressed ready to re-review.

@dangoor
Copy link
Contributor

dangoor commented Apr 23, 2014

Hey @JeffryBooher, wow... small code change!

Unfortunately, I think a bit more code change is required, though:

  1. Trying this out, the results are not ideal. Using the example from Create aliases for specific margin and padding types #5445, typing "ml" returns all sorts of stuff but margin-left is hiding way down there because the results are being sorted alphabetically. StringMatch returns too many results to be useful with an alpha sort. There's a function called basicMatchSort in StringMatch that you'd want to use to get results based on the StringMatch scoring. I'm guessing that using the match sort typing "ml" will put "margin-left" at the top.
  2. There are 3 unit test failures for this extension on this branch.

@@ -33,12 +33,15 @@ define(function (require, exports, module) {
HTMLUtils = brackets.getModule("language/HTMLUtils"),
LanguageManager = brackets.getModule("language/LanguageManager"),
TokenUtils = brackets.getModule("utils/TokenUtils"),
StringMatch = brackets.getModule("utils/StringMatch"),
PreferencesManager = brackets.getModule("preferences/PreferencesManager"),
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need that module any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good Point!

@JeffryBooher
Copy link
Contributor Author

@dangoor basicStringMatch didn't do it. I'm going to have to write a custom sorter which i had thought about doing anyway.

@JeffryBooher
Copy link
Contributor Author

It looks like jsCodeHints makes its own sort functor to callback with

penalizeUnderscoreValueCompare()

We could do something similar to that in CSS code hints

but we first want the needle to match

'ba' (background-*) matches first
and (background-attachement) is the first item in the list.

This may be difficult to do with a callback since we need the needle as well...

@dangoor
Copy link
Contributor

dangoor commented Apr 24, 2014

@JeffryBooher The stringMatch function returns a value with a "matchGoodness" score. You need to hang on to that value for all of the matches and then sort on that. The penalizeUnderscoreValueCompare is just there in JS code hints to prefer public API values to private ones. JS code hints is doing what the basicMatchSort does but customized a little bit to handle the stack depth of the match and the underscores.

@JeffryBooher
Copy link
Contributor Author

@dangoor Tests fixed and fuzzyMatching should be on par with what's expected:

"b-l-c" will get you "border-left-color" as the only option. "ml" will yield a list with "margin-left" at the top.

@dangoor
Copy link
Contributor

dangoor commented Apr 24, 2014

@JeffryBooher Works great!

The code duplication seems borderline for refactoring to eliminate the duplication, but I think it's small enough that this is fine.

I'll go ahead and merge. Thanks for the nice addition!

dangoor added a commit that referenced this pull request Apr 24, 2014
@dangoor dangoor merged commit 4fa5b3f into master Apr 24, 2014
@dangoor dangoor deleted the jeff/fuzzyCssCodeHints branch April 24, 2014 17:57
@humphd
Copy link
Contributor

humphd commented May 11, 2015

One of the commits in this introduced a regression, see #11088.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants