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

[WIP] Search #253

Merged
merged 7 commits into from
Mar 22, 2018
Merged

[WIP] Search #253

merged 7 commits into from
Mar 22, 2018

Conversation

Keats
Copy link
Collaborator

@Keats Keats commented Mar 15, 2018

No description provided.

add_section_to_index(&mut index, section);
}

index.to_json()
Copy link
Collaborator Author

@Keats Keats Mar 15, 2018

Choose a reason for hiding this comment

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

@mattico something weird is happening here. I only get title and body as key of the JSON but running the book example of mdbook gives me:

"fields": ["title", "body", "breadcrumbs"],
"pipeline": ["trimmer", "stopWordFilter", "stemmer"],
"ref": "id",
"version": "0.9.5",
"index": { // the actual index

Looking at the elasticlunr-rs code, those fields should be serialised but it's somehow not happening?

To try you can cargo build and run gutenberg build in the docs directory. A bit late here now so I will dig more tomorrow

Copy link
Contributor

Choose a reason for hiding this comment

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

mdbook creates the index with a breadcrumbs field: let mut index = elasticlunr::Index::new(&["title", "body", "breadcrumbs"]);. Does that answer your question?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not really, I just don't get the version, ref, pipeline, fields fields: the index generated by that line is just {title: {...}, body: {...}}. Way too late now though here, I'll have a better look tomorrow

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, okay, I understand the question now. The Index struct is just the index. It doesn't include any of the configuration stuff. If you take a look at MDBook, it creates that configuration by itself. I meant to add those structs to elasticlunr-rs since they're generally useful, but never got around to it. I'll take another look.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here it is: mattico/elasticlunr-rs#9 Let me know what you think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry I wasn't clear, it's not the search options that are missing.
The beautified searchindex.js of mdbook after removing the actual doc content for brevity is:

window.search = {
    "searchoptions": {
        "bool": "AND",
        "expand": true,
        "limit_results": 20,
        "teaser_word_count": 30,
        "fields": {
            "title": {
                "boost": 2
            },
            "body": {
                "boost": 1
            },
            "breadcrumbs": {
                "boost": 2
            }
        }
    },
    "index": {
        "fields": ["title", "body", "breadcrumbs"],
        "pipeline": ["trimmer", "stopWordFilter", "stemmer"],
        "ref": "id",
        "version": "0.9.5",
        "index": {
            "breadcrumbs": {
            },
            "title": {
            },
            "body": {
            }
        },
        "documentStore": {
        }
    }
};

The searchoptions object is added by mdbook as you mentioned and I don't really care about that, I'll set it up in JS directly. Here's what index.to_json gives me in Gutenberg:

window.searchIndex = {
    "body": {
    },
    "title": {
    }
};

In short I'm missing all of fields, pipeline etc from the first index object of mdbook.
The reason is that to_json does:

pub fn to_json(&self) -> String {
        serde_json::to_string(&self.index).unwrap()
}

except elasticlunr expects all those fields (https://github.com/weixsong/elasticlunr.js/blob/master/lib/index.js#L63-L81). It looks like to_json should be:

pub fn to_json(&self) -> String {
        serde_json::to_string(&self).unwrap()
}

but maybe I'm missing something.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. That seems like a rather silly oversight. I'm sure there was I reason I did it that way... but that was several months ago.

@Keats
Copy link
Collaborator Author

Keats commented Mar 20, 2018

@mattico Search is now working, just missing some design work for the docs now

If anyone wants to try it, they can clone that branch

@Libbum
Copy link
Contributor

Libbum commented Mar 20, 2018

The can_build_search_index test fails since it looks for search_index.js but it has been extended to append the default language. That needs to propagate through the examples and such.

If I enable search in the config, it complains I've turned it off in my index, but I haven't. Actually, the only way I can get it to compile is disable it in my main index file, which is the section I'd like to search.

The resultant search_index.en.js looks quite mangled as a result too:

"w":{"df":0,"docs":{},"e":{"df":0,"docs":{},"d":{"df":0,"docs":{},"e":{"df":0,"docs":{},"n":{"df":1,"docs":{"http://127.0.0.1:1111/about/":{"tf":1.4142135623730952}}}}}}},"e":{"df":0,"docs":{},"n":{"df":0,"docs":{},"t":{"df":0,"docs":{},"i":{"df":0,"docs":{},"m":{"df":0,"docs":{},"e":{"df":0,"docs":{},"n":{"df":0,"docs":{},"t"

lines and lines of this.

Attempting to search on the site (once I'd dropped in the sections from docs/templates/index.html) didn't return anything at all, no errors in the console either.

That was just a quick look though, and it was time for bed a while ago, so I'll take a closer look tomorrow.

@Keats
Copy link
Collaborator Author

Keats commented Mar 20, 2018

Thanks for the review!

I fixed the first 2 points.
The content in search_index.en.js is an inverted index so it actually does like this!

Attempting to search on the site (once I'd dropped in the sections from docs/templates/index.html) didn't return anything at all, no errors in the console either.

Even if you search for something like theme?

That was just a quick look though, and it was time for bed a while ago, so I'll take a closer look tomorrow.

I need to get some sleep too :)

@Libbum
Copy link
Contributor

Libbum commented Mar 21, 2018

Oh cool, I haven't come across an inverted index before—nice!

Won't have time to look at this again until the afternoon.

@Libbum
Copy link
Contributor

Libbum commented Mar 21, 2018

Here's the alterations I've done: 7cc3d5d.

From the looks of it these are the only changes I should need, although I don't see any updates in the search-results__item div. Did I miss something?

@Keats
Copy link
Collaborator Author

Keats commented Mar 21, 2018

Did I miss something?

You have another element with an id = search in the header so the JS binds the event handling to that one instead.
Super cool design btw

@Libbum
Copy link
Contributor

Libbum commented Mar 21, 2018

Bah! Yes, of course! Can confirm it's working now 🍾

And thanks! Revamping my old site has given me some motivation to actually post more content.

@Keats Keats merged commit ab03d26 into next Mar 22, 2018
@Keats Keats deleted the search branch March 22, 2018 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants