Support partial matches #36

nusjzx · 2018-06-08T08:48:26Z

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [ ] Documentation update
• [ ] Bug fix
• [ ] New feature
• [X] Enhancement to an existing feature
• [ ] Other, please explain:

Resolves MarkBind/markbind#218

What is the rationale for this request?
Currently when searching, only entries that match all words are shown in results,
entries that match some of the words can be shown, possibly at a lower part of the results

What changes did you make? (Give an overview)
calculate the number of matches for each entries an sort the entries by the number of matches

Testing instructions:
1.run npm run docs
2.can try key in keywords from different entries. eg: "hello bugs world"

yamgent · 2018-06-11T00:18:55Z

So now we get the OR functionality, by losing the AND functionality.

I think this feature needs further discussion, specifically whether it is useful and how we can implement it without losing what we have previously worked so hard in implementing. :P

damithc · 2018-06-11T00:26:11Z

So now we get the OR functionality, by losing the AND functionality.

If there are three search terms, results that match all three should be on top of the results. i.e., AND is still there. In addition, we also show results that match fewer search terms. We can even show which terms are not matched. e.g.,

yamgent · 2018-06-11T00:45:25Z

If there are three search terms, results that match all three should be on top of the results. i.e., AND is still there.

You are right, I forgot about that, thanks for the reminder.

That means the rank implementation is wrong then, because entries that matches multiple search terms are ranked lower:

nusjzx · 2018-06-11T04:00:51Z

@yamgent the new commit will solve the bug above

yamgent · 2018-06-11T05:21:20Z

src/Searchbar.vue

@@ -21,29 +21,42 @@ export default {
      if (!this.data) {
        return undefined;
      }
+
+      function calculateMatchNum(searchTarget, regexes) {


Move this method above if (this.value.length < 2) {

yamgent · 2018-06-11T05:22:09Z

src/Searchbar.vue

-              title,
-            });
+        let searchTarget = [title].concat(keywords).concat(Object.values(headings)).join(' ');
+        let matchNum = calculateMatchNum(searchTarget, regexes);


Rename calculateMatchNum() to getTotalMatches()

yamgent · 2018-06-11T05:30:34Z

src/Searchbar.vue

+        let searchTarget = [title].concat(keywords).concat(Object.values(headings)).join(' ');
+        let matchNum = calculateMatchNum(searchTarget, regexes);
+
+        if (matchNum) {


Might be good to be a bit more explicit in the condition (i.e. if (matchNum > 0)).

yamgent

Minor nits

yamgent · 2018-06-11T07:28:11Z

src/Searchbar.vue

@@ -15,35 +15,47 @@ export default {
  },
  computed: {
    primitiveData() {
+      function getTotalMatches(searchTarget, regexes) {
+        return regexes.reduce((matchNum, regex) => (regex.test(searchTarget) ? matchNum + 1 : matchNum), 0);


matchNum -> acc

yamgent · 2018-06-11T07:28:24Z

src/Searchbar.vue

-              title,
-            });
+        let searchTarget = [title].concat(keywords).concat(Object.values(headings)).join(' ');
+        let matchNum = getTotalMatches(searchTarget, regexes);


matchNum -> totalMatches

Why was this marked outdated?

Why was this marked outdated?

Oops, sorry, oversight on my end.

acjh · 2018-06-11T13:59:58Z

src/Searchbar.vue

      if (this.value.length < 2) {
        return [];
      }
      if (!this.data) {
        return undefined;
      }
      const matches = [];
-      const regexes = this.value.split(' ').map(searchKeyword => new RegExp(searchKeyword, 'i'));
+      const regexes = this.value.split(' ').filter(searchKeyword => searchKeyword !== '')


filter on next line.

acjh · 2018-06-11T14:07:10Z

src/Searchbar.vue

-              title,
-            });
+        let searchTarget = [title].concat(keywords).concat(Object.values(headings)).join(' ');
+        let matchNum = getTotalMatches(searchTarget, regexes);


Why was this marked outdated?

acjh · 2018-06-11T14:07:58Z

src/Searchbar.vue

-            });
+        let searchTarget = [title].concat(keywords).concat(Object.values(headings)).join(' ');
+        let matchNum = getTotalMatches(searchTarget, regexes);
+


Unnecessary empty line.

acjh · 2018-06-11T14:09:07Z

src/Searchbar.vue

-              src,
-              title,
-            });
+        let searchTarget = [title].concat(keywords).concat(Object.values(headings)).join(' ');


I don't get the point of concatenating all headings.

if not how do you wanna check isMatchingPage,

But isMatchingPage is supposed to not check for headings right? The calculation of the ranking should be independent of isMatchingPage.

const isMatchingPage = getTotalMatches(searchTarget, regexes) === matchNum;
It is comparing the totalMatches of all info(including all headings) with info (not including headings)

Ok, how about this:

this.data.forEach((entry) => { const {headings, src, title} = entry; const keywords = entry.keywords || ''; let searchTarget = [title].concat(keywords).join(' '); const pageTotalMatches = getTotalMatches(searchTarget, regexes); if (pageTotalMatches > 0) { matches.push(Object.assign(entry, { totalMatches: pageTotalMatches })); } Object.entries(headings).forEach(([id, text]) => { const searchTarget = [title].concat(keywords).concat(text).join(' '); const totalMatches = getTotalMatches(searchTarget, regexes); if (totalMatches > 0) { matches.push({ heading: {id, text}, keywords, src, title, totalMatches, }); } }); });

but this will rank the page entry lower than its heading entry.

That is fine, if it matches lesser keywords it should be lower anyway.

If it matches only title and keywords, it will show an entry for every heading?
That does not seem right — and makes the results unnecessarily long.

I'm not sure about ranking the page entry lower than its heading entries.
The page should be the main target; heading entries serve as quick links.
It seems pointless to show the page entry at the bottom.

acjh · 2018-06-11T14:48:01Z

src/Searchbar.vue

@@ -15,35 +15,47 @@ export default {
  },
  computed: {
    primitiveData() {
+      function getTotalMatches(searchTarget, regexes) {
+        return regexes.reduce((acc, regex) => (regex.test(searchTarget) ? acc + 1 : acc), 0);


Let's call it total instead of acc.

An alternative is:

regexes.filter(regex => regex.test(searchTarget)).length;

yamgent

I am good with this. @acjh is this ok for you?

yamgent suggested changes Jun 11, 2018

View reviewed changes

yamgent reviewed Jun 11, 2018

View reviewed changes

acjh reviewed Jun 11, 2018

View reviewed changes

Support partial matches

ef29d95

nusjzx force-pushed the 218-support-partial-matches branch from c7fafb5 to ef29d95 Compare June 20, 2018 03:30

yamgent approved these changes Jun 20, 2018

View reviewed changes

acjh approved these changes Jun 20, 2018

View reviewed changes

yamgent mentioned this pull request Jun 21, 2018

Vue 2.0 migration: Backport latest Searchbar features from Vue 1.0 to Vue 2.0 MarkBind/markbind#268

Closed

yamgent merged commit a9580af into MarkBind:master Jun 21, 2018

yamgent modified the milestones: Migration, v1.1.37-markbind.11 Jun 21, 2018

nusjzx mentioned this pull request Jun 28, 2018

Backport latest Searchbar features #51

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support partial matches #36

Support partial matches #36

nusjzx commented Jun 8, 2018

yamgent commented Jun 11, 2018

damithc commented Jun 11, 2018

yamgent commented Jun 11, 2018

nusjzx commented Jun 11, 2018

yamgent Jun 11, 2018

yamgent Jun 11, 2018

yamgent Jun 11, 2018

yamgent left a comment

yamgent Jun 11, 2018

yamgent Jun 11, 2018

acjh Jun 11, 2018

yamgent Jun 12, 2018

acjh Jun 11, 2018

acjh Jun 11, 2018

acjh Jun 11, 2018

acjh Jun 11, 2018

nusjzx Jun 12, 2018

yamgent Jun 12, 2018

nusjzx Jun 12, 2018

yamgent Jun 13, 2018

nusjzx Jun 13, 2018

yamgent Jun 13, 2018 •

edited

Loading

acjh Jun 17, 2018

acjh Jun 11, 2018

yamgent left a comment

Support partial matches #36

Support partial matches #36

Conversation

nusjzx commented Jun 8, 2018

yamgent commented Jun 11, 2018

damithc commented Jun 11, 2018

yamgent commented Jun 11, 2018

nusjzx commented Jun 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent Jun 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent left a comment

Choose a reason for hiding this comment

yamgent Jun 13, 2018 •

edited

Loading