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

add aliases search array and display best matches for aliases to user #30

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

rouilj
Copy link
Contributor

@rouilj rouilj commented Feb 5, 2023

Note: This upgrades the required version of fuse.js to 6.6.2. This version allows weighting to be properly specified.

This adds an aliases array for each command. A match in the alias has a weight of 2x compared to the name or description fields. If the user enables the displayHints option, the best matching alias will be appended to the end of the command name. This is useful for listing alternate terms for the one that is chosen as the command name. For example:

name: emacs, alias: editor

will show up as emacs (editor) if the user typed edit.

The README.md has more info.

This was inspired by the implementation of synonyms described at:

https://blog.superhuman.com/how-to-build-a-remarkable-command-palette/#rule-4-build-flexibility-into-your-command-palette

Note: This upgrades the required version of fuse.js to 6.6.2. This
version allows weighting to be properly specified.

This adds an aliases array for each command. A match in the alias has
a weight of 2x compared to the name or description fields. If the user
enables the displayHints option, the best matching alias will be
appended to the end of the command name. This is useful for listing
alternate terms for the one that is chosen as the command
name. For example:

  name: emacs, alias: editor

will show up as emacs (editor) if the user typed edit.

The README.md has more info.

This was inspired by the implementation of synonyms described at:

https://blog.superhuman.com/how-to-build-a-remarkable-command-palette/#rule-4-build-flexibility-into-your-command-palette
@rouilj
Copy link
Contributor Author

rouilj commented Feb 5, 2023

This addresses #8.

@rouilj
Copy link
Contributor Author

rouilj commented Feb 5, 2023

I forgot to mention that this also changes the sortFn used by fuse.js. I presort my command array by weight. I want the search results to take this into account.

If two commands are close enough to each other (< 0.05 difference in scores) they don't sort by score but by their order in the commands array. With lower indexes sorting first. 0.05 is a magic number that produced the results I wanted in a couple of tests but I didn't try to optimize it (or understand the underlying algorithm for the scores).

I think I have implemented the equivalent of / \(\w[\s\w-]*\)$/ in
unicode. So non-latin languages should work.

Also refactored the regexp out so it's not in 3 places in the file.

Thanks to accidently finding the solution on
https://www.roboleary.net/programming/2020/04/24/pimp-blog-reading-time.html.
John P. rouillard added 11 commits February 6, 2023 16:47
This seems to do better.

It weighs runs of consecuting matching characters hight. So searching
for "ab" would rank cab higher than canb.
Also change a couple of variable names to match the new name of the
former match_index function.
Removes one level of dereference from the inner loop.
Means that changes to match_metric can't change the match.

Also renamed variable that stores the metric from sum to metric.
the values used to be a simple sum of differences. It isn't anymore.
sort so entry with highest match_metric will be first. If two
match_metrics are equal, keep the order the entry is found in the
aliases array.
Simple cleanup to clarify what's happening, shorten lines, reduce
dereferencing.
I realized this patch replaced the standard fuse.js sort bunction. It
is now an option to replace the sort function. Setting
orderedCommands: true replced the default fuse.js sort function.

This also includes a new replacement sort which is better than the
original.

The original would sort two items with scores of 0.03 and 2.2E-10 by
their order in the commands array. The new function sorts by score if
the score is < 0.009. For results that are close (< 0.05 apart), the
results will be sorted by their position in the command array. This
allows the dev to nudge results when the confidence in the scores is
lower.

This also pushes the debugOutput option down into App.svelte.

In README.md, I moved the aliases and hints section lower and added a
section on ordering matches.
Copy link
Owner

@benwinding benwinding left a comment

Choose a reason for hiding this comment

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

I've been looking at this for a while, I can see the benefit and it's the whole reason I used fuse.js. I feel it could be a lot simpler though, and docs will need some work (after)

Maybe merge in the latest master if you get a chance too

src/App.svelte Outdated Show resolved Hide resolved
@@ -131,6 +131,7 @@ const c = new CommandPal({
hotkey: "ctrl+space", // Launcher shortcut
hotkeysGlobal: true, // Makes shortcut keys work in any <textarea>, <input> or <select>
placeholder: "Custom placeholder text...", // Changes placeholder text of input
displayHints: false, // if true, aliases are displayed as command hints
Copy link
Owner

Choose a reason for hiding this comment

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

Thankyou, I'm coming around to this option 👍

function removeHints(items) {
if (! displayHints ) return;
items.map( (i) => { if ( i.hinted ) {
i.name = i.name.replace(hintRegexp, '');
Copy link
Owner

Choose a reason for hiding this comment

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

So this removes the hint if they have used it? Maybe a comment above here?
If so that's awesome 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this removes the hint if they have used it? Maybe a comment above
here? If so that's awesome +1

It just resets the command palette names to remove hints. It doesn't matter if they are
used or not. It returns the commands array name properties to their original state for
the next invocation.

This way pulling up the palette doesn't display the hints from the last invocation before the user even types anything.

LGTM changing the code now.

Co-authored-by: Ben Winding <[email protected]>
@rouilj
Copy link
Contributor Author

rouilj commented Feb 11, 2023

I've been looking at this for a while, I can see the benefit and it's the whole reason
I used fuse.js. I feel it could be a lot simpler though, and docs will need some work (after)

If there is a way to make fuse.js apply a weight to an object, I didn't see it.
All I saw was weights on the fields in the object.

Maybe merge in the latest master if you get a chance too

Working on it now.

@rouilj rouilj requested a review from benwinding February 11, 2023 03:21
@rouilj
Copy link
Contributor Author

rouilj commented Feb 11, 2023

I think I got this right. I don't use git or GitHub much. My preferrd DVCS is Fossil because well, I'm a fossil 8-). Take another look.

Also nice use of ChatGpt. what did you feed it to get it to generate the refactored code?
It may not be as performant, but it certainly is much easier. If I had written it as refactored
I would probably have finished writing it much more quickly 8-).

@benwinding
Copy link
Owner

I think I got this right. I don't use git or GitHub much. My preferrd DVCS is Fossil because well, I'm a fossil 8-). Take another look.

Looks good, I'll fix the type errors in the next few days, probably makes sense to put this in a filtering.ts file I think

Also nice use of ChatGpt. what did you feed it to get it to generate the refactored code? It may not be as performant, but it certainly is much easier. If I had written it as refactored I would probably have finished writing it much more quickly 8-).

It's amazingly flexible this, you can be pretty expressive and just paste code in below it

Can you make this JS code a bit easier to read, remove the comment, and add named boolean variables before returning:
(and then pasted code below it)

@rouilj
Copy link
Contributor Author

rouilj commented Feb 11, 2023

One thing I would leave the comment that 0.05 and 0.009 are magic numbers with little if any research to justify them.

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

Successfully merging this pull request may close these issues.

2 participants