Doc sherlodoc #1086
Doc sherlodoc #1086
Conversation
This changes driver.mld to use sherlodoc. The database is 11MB large. Sherlodoc's sources must be cloned into the dune workspace in order for it to use the same version of Odoc as the driver. Installing it via Opam results in magic number clashes.
|
I removed this from draft because the concern I had is not a big deal. If you have sherlodoc linked to the wrong odoc, there wont be a diff in the driver, because its the content of the .js file that will be different. |
There was a problem hiding this comment.
Thanks!
I left some minor comments.
This should probably explained and documented, but it is not a blocker for merging.
I think it is a blocker for merging! I tried to make it work (dune -p odoc @install then opam install sherlodoc then dune build @docgen) and failed.
| Finally, we generate an index of all values, types, etc. This index is meant to be consumed by search engines, to create their own index. It is in json format. Generating the index is done via [odoc compile-index], which creates a json file. | ||
|
|
||
| Some more details about the json format: | ||
|
|
||
| {ul | ||
| {- The whole file is a json array.} | ||
| {- each entry of the array corresponds to an item to search. An entry is a json object, with the following fields: | ||
| {ul | ||
| {- ["id"], which is a string. It corresponds to the id of the entry, for instance: ["Stdlib.List.map"].} | ||
| {- ["doc"], which is a string. It corresponds to a "searchable" version of the docstring: one which is stripped from any html or specific markup. This is to avoid having too many results when searching ["class"], for instance.} | ||
| {- ["kind"] is a json object. It contains the fields: | ||
| {ul | ||
| {- ["kind"], a string encoding the kind of the entry (type, value, module, standalone comment, etc.)} | ||
| {- any information that is specific to it (for instance, a type has kind ["TypeDecl"] and also contains the constraints, manifest, and whether the type is private or not). For a comprehensive description of what information is available in this field, please refer to the source file: src/search/json_index/json_search.ml}}} | ||
| {- ["display"], which is a json object. It contains two fields: | ||
| {ul | ||
| {- ["url"], a string. It is the URL to the entry in the documentation, relative to the base of the documentation} | ||
| {- ["html"], also a string. It is the html odoc uses to display the entry in the search results.}}}}}} |
There was a problem hiding this comment.
This documentation should not simply be removed, but moved somewhere else. Maybe in interface.mld?
| We turn the JSON index into a javascript file. In order to never block the UI, this file will be used as a web worker by [odoc], to perform searches: | ||
|
|
||
| - The search query will be sent as a plain string to the web worker, using the standard mechanism of message passing | ||
| - The web worker has to sent back the result as a message to the main thread, containing the list of result. Each entry of this list must have the same form as it had in the original JSON file. | ||
| - The file must be given to the [odoc-support] URI. |
There was a problem hiding this comment.
This doc should also remain somewhere
| |> List.filter (fun name -> | ||
| try (ignore (Str.search_forward regexp_dunder name 0); false) | ||
| with Not_found -> true ) |
There was a problem hiding this comment.
Isn't it the same as
| |> List.filter (fun name -> | |
| try (ignore (Str.search_forward regexp_dunder name 0); false) | |
| with Not_found -> true ) | |
| |> List.filter (fun name -> not (String.contains "__" name) |
?
There was a problem hiding this comment.
This signature of contains is val contains : string -> char -> bool
There was a problem hiding this comment.
Ah yes. In this case we use Astring.String.is_infix. but actually this is already implemented as is_hidden (line 479), so let's not duplicate the logic!
| let opam_switch_prefix = Astring.String.Map.get "OPAM_SWITCH_PREFIX" env | ||
|
|
There was a problem hiding this comment.
I think this can be removed:
| let opam_switch_prefix = Astring.String.Map.get "OPAM_SWITCH_PREFIX" env |
I think @install build the required file that should be installed, its "dune install" taht actually install them. Regardless, it should not fail. I said it should not block merging because it thought it would not fail, but instead just produce a wrong output, but apparentyl this is false. |
|
I am closing this because the driver has been completely rewritten. |
This is two things rebases #1064, and update the sherlodoc interface to the definitive one released in sherlodoc.
This gives the benefits that stdlib results are not shown above odoc ones anymore.
It would be a bad idea to merge this because the driver would be hard to run properly: you need to have sherlodoc installed and linked to the current odoc version which is challenging during development.