Support for search in `odoc` #972

panglesd · 2023-06-09T07:25:07Z

This PR adds support for search in odoc.

It leaves the actual search to an external search engine, but exposes the information needed for such a search engine to work, and defines the communication between the search engine and odoc’s page.

It is not very polished yet, but close, I open the PR to start the review, and discuss some design choices.

More precisely, here is what the PR is about:

The addition of a Odoc_model.Fold module, folding on the values of type in Lang (can be useful to generate a glossary of values/modules/... too).
The addition of a type, Odoc_search.Entry.t. A value of this type correspond to an entry in a search index. This type is supposed to stay stable as it can be consumed by search indexes written in OCaml (to still allow for breaking modification of the Lang module). Entries can be printed as a JSON object (sometimes simplified).
This PR specifies the communication between search engines and odoc, through the json file consumed by search engines (in json_search.ml) and the json outputed by the search engine (in json_display.ml)
The modification of the doc-comments and mld pages, to have them include an id on basic blocks of text (paragraphs, "leaf" list items, verbatim blocks, ...), so that they are searchable/linkable.
An option not to render links or html id/anchors, to avoid duplicated IDs or links inside links, when rendering the search results.
On the generated html pages: The execution of the search engine "front-end code" as a webworker, and the display of the results (the code for this needs to be reworked a bit).

And, in terms of CLI:

The addition of the compile-index command, which outputs a JSON file, meant to be read for indexation by external search engines
~~The addition of a --search-file <path> option to support-files. This option copies the given file in the relevant directory.~~
The addition of a --search-file <name> option to html-generate. This option activates the search elements on the generated pages (text input and result area), and includes the loading of the name file (which has to be included with the corresponding option of support-file). replaced by:
The addition of a --search-asset asset_reference option to compile. This option tells to use the given asset to load as a search script.

Finally, to be able to test:

An update of the driver to provide client-side search, using the minisearch library.
Some tests.

You can view the result of running the driver directly here. (The minisearch library sometimes behaves strangely, but it is just an example, for reference).

TODOs/Improvements:

Paragraphs and basic blocks of text have their ID disambiguated at link stage (the href needs to be fully knowable after the link stage, for the index to have the right URL). However, headings still have their ID disambiguated at the document level. Need to change that to have the correct URLs for headings in the search index.
Add labels to basic text block (such as paragraphs and code blocks) #974 was opened, but the decision was to start simple: just link to the page containing the paragraph, for now
@EmileTrotignon has reported seing some internal values being indexed in his tests. Need to find an example and fix.
@EmileTrotignon has reported many Warning, resolved hidden path: Base__.Set_intf.Named.t being output when indexing.
The same warning appear on html-generate of the culprit documents. Not introduced in this PR, and not a big deal. Considered done.
The rendering of search bar and results could improved with advices from webdesigners? The code for it could be improved too: The situation being different, I think we should write a new generator that takes Entry.t and generates html, and which does not depend on Odoc_html.
I consider this "done": The code has been improved, and the CSS is satisfying to me (see this comment)
Starting the webworker only when the user click on the search bar might save a lot of computing resources.
Makes it so that the information in a JSON entry and an Odoc_search.Entry.t are the same.
I think there is enough info in the JSON entry. Marking it as done.
Benchmark (time and space) and add more tests.
I think the test is now sufficient.The compile-index is almost immediate on odoc + all of its dependencies. The efficiency of the search engine is not part of this PR.
Rebase.

Separating the search engine from `odoc`

As hinted above, the actual search engine is supposed to be external from odoc.

It can generate its database from a JSON file (whose format is defined int the src/search/json_search.ml file), or by using odoc as a library.

It should a javascript file that will be run as a webworker. This webworker should listen to"webworker messages", each message being a query (given as plain string) to search for.

The result of a queried search must be an array of json object of type defined in /src/search/json_display.ml. An odoc-generated value for each entry is included in the input of the search engine, which is welcome to modify it/generate its own, for instance to highlight the reason this result was chosen.

As the search script is in a webworker, the result of the search has to be sent back to the main thread via postMessage, and odoc will render the results.

This allows to have different scenarios:

Client-side search using a js library (possibly compiled with jsoo). Good for search restricted to a single package. However, the whole search index and search logic is sent to the client on each load page. dune build @doc could use that.
Server-side search. The index.js file simply does a request to an API. Can search on much bigger dbs, using any search engine (elasticsearch, a custom OCaml-specific search engine, ...).

odoc could also depend on a blessed search engine (sherlodoc), to allow for a more "out-of-the-box" search support.

Sherlodoc

Sherlodoc has already been modified by @EmileTrotignon to use the functionality added by this PR to generate the sherlodoc search index.
It can be compiled to javascript using jsoo, to have client-side search on single libraries, or be used server-side for bigger indexes.
Compared to the current web version (doc.sherlocode.com), the modified version also search inside doc comments, and is not restricted to values (it can search module, module types, constructors, ...).

The corresponding branch is jsoo-compat.

Pinging also @art-w !

Data to build the search index

The data to build the search index can be read in the JSON file generated by the compile-index command. However, the code is made so that it is also easy to use odoc as a library, and use values of type Odoc_search.Entry.t (that you get by combining Odoc_model.Fold with Odoc_search.Entry.of_{value/type/...}).

Each have its advantages and use-case:

The JSON format is readable by most programming language and search engines. It is easily extensible with new fields without breaking the consumers of this format.
The OCaml Entry.t format encodes slightly more information, and is more practical to use by OCaml programs.

The Entry.t type is defined in Odoc_search.Entry.t. The JSON format is defined in Odoc_search.Json_search. An example of a JSON entry would be:

    {
      "id": [
        {
          "kind": "Root",
          "name": "Main"
        },
        {
          "kind": "Value",
          "name": "lorem"
        }
      ],
      "doc": "lorem 1 and a link",
      "extra": {
        "kind": "Value",
        "type": "int"
      },
      "display": {
        "html": "<div class=\"search-entry val\"><code class=\"entry-title\"><span class=\"entry-kind\">val</span><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">lorem</span><code class=\"entry-rhs\"> : int</code></code><div class=\"entry-comment\"><div><p>lorem 1 and a <span>link</span></p></div></div></div>",
        "url": "Main/index.html#val-lorem"
      }
    }

It contains:

A JSONification of the ID,
A stringified version of the docstring
An extra field containing:
- The kind of entry
- All extra information on this kind of entry. For instance, for values, the type of the value is included (as a string). The set of extra information per entry kind is available in src/search/json_output.ml
A display field, corresponding to the JSON value to return in case of inclusion of this entry in the search results:
- The url to the entry (relative to the base of the compilation unit, which is known in the page executing the search)
- a html field, containing how the html to use to display the entry in the search result.

I am not sure the intermediate representation of a search entry, Odoc_search.Entry.t, is a good design. I would be curious about your opinion!

Running as a webworker

Running as a webworker is nice to isolate the search engine, and prevent it to freeze the UI at any point.
The browsers interpretation of the CORS origin policy prevents to run webworkers from javascript files fetched from the file:// protocol. I used a hack to go around this restriction.

dbuenzli · 2023-06-09T09:25:34Z

Looks nice!

defines the communication between the search engine and odoc’s page.

I think it would be better to simply have a search.js file living in the assets directory that is simply added to each page (in the same vein as highlight.js is).

This script can modify the dom to add the search interface and define the interaction with the search engine the way it wishes. This abstracts away all the webworker stuff, basically pages are completely agnostic to how search works.

This would allow themes to redefine search they way they see it fit.

I am not sure the intermediate representation of a search entry, Odoc_search.Entry.t, is a good design. I would be curious about your opinion!

It looks ok to me. Maybe I would rename the Doc case to Text or Unattached or Standalone, there's too much docs in odoc (though I understand that's how it appears in the document model of odoc).

Regarding the html for result snippets. I'm not exactly sure what you did put here. I would suggest to simply put a marked up version of text.doc which corresponds to the rendering pointed to by the url and let the presenter add the classes it needs. An indexer may actually want to skip the field and rather store byte offsets into the file pointed to by url.

dbuenzli

Had a cursory look and left a few comments.

dbuenzli · 2023-06-09T09:27:49Z

src/html_support_files/odoc_search.js

+  URL.revokeObjectURL(blobUrl);
+
+  return worker;
+}


Welcome to the ridiculous world of web programming. Been there. You should add a comment that you are making that dance to enable webworking over the file:// protocol.

dbuenzli · 2023-06-09T09:33:06Z

src/search/json_display.ml

+          else None
+      | Constructor { args; res } ->
+          Some (" : " ^ display_constructor_type args res)
+      | Field { mutable_ = _; type_; parent_type = _ } ->


If you take the habit of priming (') identifiers that conflict with keywords rather than suffixing them with _ these kind of patterns look much less confusing:

| Field { mutable' = _; type'; parent_type = _ } ->

In the current code, _ is always used to escape keywords. There's one occurrence of module', which is a internal function used by module_.

dbuenzli · 2023-06-09T09:53:34Z

src/model/semantics.ml

+    incr current_label;
+    Paths.Identifier.Mk.label
+      ( status.parent_of_sections,
+        Names.LabelName.make_std ("search_label_" ^ string_of_int !current_label)


That's not a very nice label. People will start to link to these things so they will show up in URLs. Can we maybe find a shorter scheme and perhaps give a better hint at the kind of element we are linking to ? Maybe something like p%d, code%d, math%d or whatever better scheme you can come up with.

jonludlam · 2023-06-09T12:38:29Z

src/xref2/component.mli

@@ -432,9 +432,21 @@ and Substitution : sig
 end

 and CComment : sig
+  type nestable_block_element =


Do we need to redefine this here? It looks identical to nestable_block_element in model/comment.ml.

Oh I see, it's because it's referencing Label.t here.

jonludlam · 2023-06-09T12:44:08Z

src/xref2/link.ml

-      (* Only the first element is considered. *)
-      Comment.synopsis [ e ]
+  | { value = `Paragraph (_, text); _ } :: _ -> Some text
+  (* | ({ value = #Comment.nestable_block_element; _ } as e) :: _ -> *)


What's happening here?

Before, the Component version of nestable block comments were identical to the Lang version. Now, they differ, so we cannot anymore call the Lang function.

The old code now commented slipped in my staged area, I will clean it.

jonludlam · 2023-06-09T12:54:16Z

I wonder if we could extract some of the markdown renderer to allow rendering of the doc comments as markdown (in Search.Render)? It's probably not going to make it much bigger, it can still be indexed and the search tool could then show a preview.

panglesd · 2023-06-09T14:16:46Z

Thanks for the very prompt review!

I think it would be better to simply have a search.js file living in the assets directory that is simply added to each page (in the same vein as highlight.js is).

This script can modify the dom to add the search interface and define the interaction with the search engine the way it wishes. This abstracts away all the webworker stuff, basically pages are completely agnostic to how search works.

This would allow #826 to redefine search they way they see it fit.

I think that it is already working similarly to this:

There is a odoc_search.js file that is added to each page. It modifies the dom, and defines the integration with the search engine. This integration is: the search engine has to provide a file which listens for message queries and output results.
Odig (or ideally odoc...) can replace this file by something else for "search theming".

Before I started using webworkers, the integration between search engine and odoc was: "the search engine has to provide a function called "odoc_search", taking a string as input, and outputing a list of results".
I could go back to this interface, while keeping it in a webworker, maybe it makes more sense to provide a function than all these details about webworkers.

(I hope I did not miss your point!)

I would suggest to simply put a marked up version of text.doc which corresponds to the rendering pointed to by the url and let the presenter add the classes it needs.

I was hesitant between two approaches:

Have completely specific snippets for search results, with a custom generator.
Have search results that matches exactly the rendering pointed by the url.

In this not finished work, it is a bit inbetween, but leaning toward the first option...

An indexer may actually want to skip the field and rather store byte offsets into the file pointed to by url.

I'm not sure how this could work, but that's clearly some bytes saved!

I wonder if we could extract some of the markdown renderer to allow rendering of the doc comments as markdown (in Search.Render)? It's probably not going to make it much bigger, it can still be indexed and the search tool could then show a preview.

That's also a possibility, that removes the need to store two versions of the docstring (the text one and the html one). But I think I prefer one of the option above: either match exactly the fully rendered version, or be tailored to be showed on a search result.

dbuenzli · 2023-06-09T15:19:37Z

This integration is: the search engine has to provide a file which listens for message queries and output results.

I'd suggest you don't even go as far as defining this. Basically you shouldn't have to touch the generator (this should not be needed). Just let search.js inject the markup for the search interface.

the integration between search engine and odoc was: "the search engine has to provide a function called "odoc_search", taking a string as input, and outputing a list of results".

I think this integration should be: include the javascript file(s) of my theme in the generated page. odoc's default theme should basically have what you provided here but with everything handled by odoc_search.js, including injecting the search form in the page. That way the search engine interface becomes totally orthogonal to odoc, the interface couldn't be simpler.

In this not finished work, it is a bit inbetween, but leaning toward the first option...

I would rather lean towards the second one. I would like html to be, as far as the text is concerned, a marked up version corresponding to doc. Possibly without all the links (but with the same kind of markup structure to also save a bit on the css). Additional wrappers can be added by odoc_search.js for the list presentation.

The reason is that I want to be able to do query term highlighting in them (you don't necessarily show the whole html, you might ellide some parts). Seeing keywords in context makes it easier for users to evaluate whether a result matches their information need. But if you start highlighting stuff that is different once you get on the actual page it becomes deceptive.

I'm not sure how this could work, but that's clearly some bytes saved!

When you retrieve the result you can just lookup the files via url, extract the byte range (a bit faster than looking up for the id) and use that for the result snippet. Rather than having them as stored fields in your inverted index. It's slower but it's a retrieval speed/storage space tradeoff.

That's also a possibility, that removes the need to store two versions of the docstring (the text one and the html one). But I think I prefer one of the option above: either match exactly the fully rendered version, or be tailored to be showed on a search result.

Your search index will not store two versions of the doc string. It will use the doc field to construct the inverted index and have url and html (or byte offsets) as stored fields for the entries.

I think it's good to have doc as proposed, it avoids the need to parse the data to extract text (something you would also need to do if that was markdown).

Having html for the result snippets allows to have the same renderings as will show up once you follow the results. I don't see any advantage in doing it in markdown: you'd have to render it at some point and thus invoke a non-trivial markdown parser/renderer and the results wouldn't match odoc's richer rendering structure.

panglesd · 2023-06-12T13:48:14Z

Thanks for the explanations!

About the "include the javascript files of my theme", I think this would be a useful feature, that I would be keen to implement (but, in another PR). It is much more general than "search".

However, I'm not sure that the inclusion of the search bar in the HTML should be done by a script by default. Most of the time I don't think the theme should be in charge of adding HTML elements. When the general layout really has to be modified, there is a json output command, which breaks the html of each UI element in different fields.

In any case (supposing the "include the javascript files of my theme" command exists) the workflow you suggest is still possible: The search bar is not included in the HTML when no --search-files argument is added to the html-generate command. The javascript files of your theme could inject a search bar where in the html it wants.

About how to render search results:

I agree with you that having consistency between how entries are displayed in the search results, and how they are displayed in the generated docs, is an important property to have.

So, I will modify this PR so that the entry.display.html field is exactly the html generated by odoc (without links and ids).

trefis · 2023-06-12T15:38:02Z

The addition of a --search-file <path> option to support-files. This option copies the given file in the relevant directory.

The addition of a --search-file <name> option to html-generate. This option activates the search elements on the generated pages (text input and result area), and includes the loading of the name file (which has to be included with the corresponding option of support-file).

I'm not keen on this; support files doesn't seem like the right place. For instance, I think I might want to have dune index each package of my workspace separately. In which case, the index looks more like an asset than a support file.

PS: having the same option name when one of them expect a path and the other one a "name" (basename?) seems like a mistake.

Julow · 2023-06-12T15:55:42Z

The modification of the doc-comments and mld pages, to have them include an id on basic blocks of text (paragraphs, "leaf" list items, verbatim blocks, ...), so that they are searchable/linkable.

The diff is huge in part because of this. What do you think of moving that to a different PR ? That could make the history easier to understand and the whole easier to review.

dbuenzli · 2023-06-12T16:25:21Z

The search bar is not included in the HTML when no --search-files argument is added to the html-generate command.

But then I don't get the link to the index file no ?

panglesd · 2023-06-20T16:39:09Z

What do you think of moving that to a different PR ? That could make the history easier to understand and the whole easier to review.

I rewrote the history and opened a new PR for the addition of IDs to block of texts.

If we want to split the conversation even further, I can open other PRs for separate features, such as the Odoc_model.Lang traversal.

I haven't yet implemented the review comments to the main commits ("Adding search support in odoc"). In particular, the way to specify the javascript files to include, and the html in the json index will change.

EmileTrotignon · 2023-06-29T12:57:49Z

I would rather lean towards the second one. I would like html to be, as far as the text is concerned, a marked up version corresponding to doc. Possibly without all the links (but with the same kind of markup structure to also save a bit on the css). Additional wrappers can be added by odoc_search.js for the list presentation.

The reason is that I want to be able to do query term highlighting in them (you don't necessarily show the whole html, you might ellide some parts). Seeing keywords in context makes it easier for users to evaluate whether a result matches their information need. But if you start highlighting stuff that is different once you get on the actual page it becomes deceptive.

I think thats a good principle, but it does work in every case. For instance, we want constructors to be searchable, and the html of constructors is not structured in a good way for that.
First of all, constructors (and fields) do not have a "kind" in the mli, and we want to display the kind of every entry ("cons" for constructors).
Especially because we want constructors to be searchable by type (this is present in sherlodoc).
This means that a constructor Hoo of goo from type foo will be type-searchable like a value goo -> foo .
We also probably want to display it as such in the search results, and if we have highlighting in the feature, to highlight the type.

That means that we want to display the following :

cons Hoo : goo -> foo

instead of

foo.Hoo of goo

And I do not think the odoc html generator is flexible enough to provide the little functions that would be necessary to get the desired output, from what I remember there is no "constructor printer", just a typedecl one, and it is not exactly easy to extract the required functions (here the function that prints "goo" is not the typeexpr printer, as "goo" may be an inline record).

In my opinion we may try two things :

Make the html generator modular enough
2, Rewrite part of it for the search results.

panglesd · 2023-07-13T10:38:57Z

Here is a summary of the changes that happened in this PR:

I removed the commits related to adding an ID to blocks of texts. Links to standalone comments/comments in mld pages now link to the page in which the comment is. This can be improved in a later PR, I think.
I now find the search script associated to a page by giving an asset reference to it. The asset reference is resolved by looking up in the current page if the asset is there, and if not, recursively looking up in the parent page. This was needed (in contrast with using support-files) when multiple pages use the same support files, but a different script for search (due to not having the same database).
The html rendering of search entries has been widely improved by @EmileTrotignon, but it is still not 100% finished as far as I understand.

I think that the followin can be reviewed:

the CLI design,
the Lang fold function,
the format of the json output

The points above consist in all the "user facing part", I think, so having it right is the most important for merging this PR!

Signed-off-by: Paul-Elliot <peada@free.fr> Co-authored-by: Jules Aguillon <jules@j3s.fr>