Search with Elasticlunr #472

Phaiax · 2017-10-22T13:57:04Z

Belongs to Issue #51 , Proof of concept of mdbooks book and of the rustbook

This PR is not ready for merging, but I wanted to allow reviews.

Tracking:

Indexing not working properly (uses workaround, investigation needed)
Configuration (waiting on Making configuration more flexible #457)
Use keys (up/down) for selecting a search result
Disabled search must disable search index generation, currently it is simply not written into the indexfile.
Measure slow down
Bug: Escape scrolls to top.
Won't do: Improve teaser generation speed

Michael-F-Bryan · 2017-11-11T06:06:38Z

@Phaiax, would you be able to add a link to your proof-of-concept version in the description so people can see what the user-facing search will be like?

mattico · 2017-11-13T03:21:50Z

@Phaiax I've updated elasticlunr-rs, and the generated indexes actually work now.

However, for search to not miss any results we need the stemmer used to generate the index to be identical to the one used to search. The version of elasticlunr-rs on crates.io uses a stemmer from rust-stemmers, which doesn't quite have the same output. The version on master on the repo uses a port of the stemmer from elasticlunr.js, but there's a bug where it doesn't work on words ending in "ted" or "ting". Until I get that fixed there may be results missing if you use generated indexes.

Phaiax · 2017-11-13T21:27:03Z

@mattico: Nice (I saw your comment in the reddit What's everyone working on thread :) )

Today I added configurability. Tomorrow I will update the proof of concept and do the key up/down handler.

One problem appeared. My teaser algorithm is way too slow and needs a rework. In the above example, it did not matter, but it is causing lags in the official rust book.

Michael-F-Bryan · 2017-11-14T10:46:49Z

@Phaiax, I just had a look at the configuration stuff and it seems pretty good so far.

Something that occurred to me is whether we want to have search enabled by default or not (e.g. enabled is true in Search::default()). If it ends up being pretty slow or expensive then users may prefer to have search deactivated while developing.

Michael-F-Bryan · 2017-11-14T10:49:40Z

src/renderer/html_handlebars/hbs_renderer.rs

+    /// The searchoptions for elasticlunr.js
+    #[derive(Serialize)]
+    struct SearchOptions {
+        bool: String,


Just out of curiosity, what does this guy do?

This mimics the structure of the searchconfig object, that elasticlunr expects. At first i serialized the config::Search struct, but that uses kebab-case which is not nice and would cause hidden breakage if modified.

Ah fair enough, so it's a elasticlunr-specific thing then. I thought bool was an unusual choice for a variable in a Rust program, given it's also the name of a primitive type.

Phaiax · 2017-11-14T21:08:46Z

I updated the previews. Links in first post.

Phaiax · 2017-11-14T21:10:06Z

I will measure the slowdown.

mattico · 2017-11-15T03:16:31Z

Oh, and I renamed the package (but not the crate!) to "elasticlunr-rs".

Phaiax · 2017-11-20T21:27:21Z

Almost done. (Commit when elasticlunr-rs 0.2.1 is pushed to crates.io)

Timing with the rust book, second edition:

Search	Debug	Release
Enabled	52s	2.2s
Disabled	24s	0.86s

You decide if it should be enabled by default, but I think it should. For small books it does not make a difference and for large books such as rust book, they will probably tell their contributors about this option.

…gs for fonts and png.

…use nightly requirement)

…light search terms.

* remove searchindex feature (nightly requirement of elasticlunr-rs dropped) * some documentation * refactor BookItems iterator * add iterator for parents * Include paragraph structure in hierarchy * Fix url and specialchar handling * Use complete index

…if search is disabled

…d add comments

Phaiax · 2017-11-26T18:08:32Z

@Michael-F-Bryan this is ready to merge.

…h is disabled.

Phaiax · 2017-11-26T18:54:38Z

Added two more things, but now it should be ready.

Michael-F-Bryan · 2017-11-27T13:17:54Z

@Phaiax, what are your thoughts on adding a couple tests to this? We'll want to ensure search index generation doesn't accidentally get broken if those sections get refactored later on. Would it make sense to add a couple integration tests for search under the tests/ directory?

Michael-F-Bryan · 2017-12-13T03:42:28Z

Sorry @Phaiax, it looks like merging #491 broke a fair amount of this PR due to conflicts 😞

We can probably remove most of the changes this PR makes to the book's internal representation because they will have (mostly) been dealt with in #491. So this PR "only" needs to worry about the html renderer and adding a Search struct to the HtmlConfig.

Phaiax · 2017-12-14T21:43:58Z

@Michael-F-Bryan I'm still planning to do this, but I have limited time in the moment

Michael-F-Bryan · 2017-12-15T08:17:58Z

@Michael-F-Bryan I'm still planning to do this, but I have limited time in the moment

No stress 🙂 I'd found myself with a lot of spare time recently and was just going through any outstanding PRs and issues, seeing if there's anything I can do to help them along.

I really like this PR and your proof-of-concept looks awesome, so if there's anything I can do to help don't hesitate to let me know!

mattico · 2018-01-28T04:25:26Z

I'm working on updating this PR: https://github.com/mattico/mdBook/tree/search-eljs-rebase

It's mostly complete, but there are a few JS & styling bugs, and the index is missing some things.

@Phaiax, if you modified the stylus files, could you push your changes? It would help me out a lot.

cetra3 · 2018-02-01T04:19:00Z

@mattico, I've got a couple of css/js changes to bring it inline with the original. I've pushed this to this branch here: https://github.com/cetra3/mdBook/tree/search-eljs-css

mattico · 2018-02-02T00:12:03Z

@cetra3 Moving the searchbar into the header was deliberate, I thought it worked better with the sticky header that we have now. Of course, then the results shouldn't be stuck into the top of the document... Yeah I'll change it back to how it was, we can have style discussions after this is merged. Thanks for the fixes!

cetra3 · 2018-02-02T00:15:52Z

@mattico yeah it can be changed. I was just having a hard time lining up the search input with the search text, as there is some indentation with the icons.

Michael-F-Bryan · 2018-03-05T08:54:24Z

@mattico and @Phaiax this has been succeeded by #604, is it okay to close this PR?

mattico · 2018-03-05T16:41:34Z

I'd say yes.

Phaiax force-pushed the search-eljs branch from 33a263f to 40340e4 Compare October 22, 2017 13:57

Phaiax mentioned this pull request Nov 8, 2017

Have a search bar #51

Closed

Phaiax mentioned this pull request Nov 12, 2017

Watch command for book.toml changes #486

Closed

Phaiax force-pushed the search-eljs branch from 40340e4 to cc008fd Compare November 13, 2017 21:20

Michael-F-Bryan reviewed Nov 14, 2017

View reviewed changes

Phaiax force-pushed the search-eljs branch from ffe3f4a to 778f147 Compare November 21, 2017 19:29

Keats mentioned this pull request Nov 22, 2017

Add search index creation getzola/zola#140

Closed

Phaiax added 12 commits November 26, 2017 19:06

Fix with .gitattributes: Git on Linux tried to autoconvert line endin…

b1d3b96

…gs for fonts and png.

Search: Add UI, elasticlunr.js, demo by indexing current page

893dc39

Render markdown into searchindex for elasticlunr. (feature gated beca…

18a1dc0

…use nightly requirement)

Search: Move results to the right, fix mobile, history handling, high…

379c6ff

…light search terms.

Search: Refactor, refine history behaviour, add breadcrumbs

c487a95

Search: Style the other themes, emphase active header.

aa1f02f

Search: Fine tuning

a198e99

* remove searchindex feature (nightly requirement of elasticlunr-rs dropped) * some documentation * refactor BookItems iterator * add iterator for parents * Include paragraph structure in hierarchy * Fix url and specialchar handling * Use complete index

Search: Improve search result snippets. Cancel highlight with ESC.

850df09

Search: Configuration via book.toml

4dcba19

Search: Result navigation with up/down

26e16a8

Search: Update elasticlunr-rs. Remove old code. Don't generate index …

66ae6b6

…if search is disabled

Search: Fix scroll-to-top on escape (unmark). Rearrange javascript an…

fa88440

…d add comments

Phaiax force-pushed the search-eljs branch from 778f147 to fa88440 Compare November 26, 2017 18:07

Search: Add config documentation. Add to contributors.

0ad8535

Search: Add handlebars variable and do not load js libraries if searc…

592fb17

…h is disabled.

This was referenced Jan 2, 2018

Customizable popup widgets #62

Closed

Feedback for upstream mdbook rust-lang/book#1092

Closed

Add support for alternative backends #507

Merged

sorin-davidoi mentioned this pull request Jan 23, 2018

WIP: Make available offline using Service Worker #571

Closed

mattico mentioned this pull request Feb 2, 2018

Search with Elasticlunr, updated #604

Merged

Michael-F-Bryan closed this Mar 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search with Elasticlunr #472

Search with Elasticlunr #472

Phaiax commented Oct 22, 2017 •

edited

Loading

Michael-F-Bryan commented Nov 11, 2017

mattico commented Nov 13, 2017

Phaiax commented Nov 13, 2017

Michael-F-Bryan commented Nov 14, 2017

Michael-F-Bryan Nov 14, 2017

Phaiax Nov 14, 2017

Michael-F-Bryan Nov 16, 2017

Phaiax commented Nov 14, 2017

Phaiax commented Nov 14, 2017

mattico commented Nov 15, 2017

Phaiax commented Nov 20, 2017

Phaiax commented Nov 26, 2017

Phaiax commented Nov 26, 2017

Michael-F-Bryan commented Nov 27, 2017

Michael-F-Bryan commented Dec 13, 2017

Phaiax commented Dec 14, 2017

Michael-F-Bryan commented Dec 15, 2017

mattico commented Jan 28, 2018 •

edited

Loading

cetra3 commented Feb 1, 2018

mattico commented Feb 2, 2018

cetra3 commented Feb 2, 2018

Michael-F-Bryan commented Mar 5, 2018

mattico commented Mar 5, 2018

Search with Elasticlunr #472

Search with Elasticlunr #472

Conversation

Phaiax commented Oct 22, 2017 • edited Loading

Michael-F-Bryan commented Nov 11, 2017

mattico commented Nov 13, 2017

Phaiax commented Nov 13, 2017

Michael-F-Bryan commented Nov 14, 2017

Michael-F-Bryan Nov 14, 2017

Choose a reason for hiding this comment

Phaiax Nov 14, 2017

Choose a reason for hiding this comment

Michael-F-Bryan Nov 16, 2017

Choose a reason for hiding this comment

Phaiax commented Nov 14, 2017

Phaiax commented Nov 14, 2017

mattico commented Nov 15, 2017

Phaiax commented Nov 20, 2017

Phaiax commented Nov 26, 2017

Phaiax commented Nov 26, 2017

Michael-F-Bryan commented Nov 27, 2017

Michael-F-Bryan commented Dec 13, 2017

Phaiax commented Dec 14, 2017

Michael-F-Bryan commented Dec 15, 2017

mattico commented Jan 28, 2018 • edited Loading

cetra3 commented Feb 1, 2018

mattico commented Feb 2, 2018

cetra3 commented Feb 2, 2018

Michael-F-Bryan commented Mar 5, 2018

mattico commented Mar 5, 2018

Phaiax commented Oct 22, 2017 •

edited

Loading

mattico commented Jan 28, 2018 •

edited

Loading