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

Fix href for aliased modules in search results #1108

Merged
merged 5 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@
- Fix missing katex headers (@panglesd, #1096)
- Fix resolution of module synopses in {!modules} lists that require
--open (@jonludlam, #1104}
- Fix top comment not being taken from includes often enough (#1117, @panglesd)
- Fix top comment not being taken from includes often enough (@panglesd, #1117)
- Fixed 404 links from search results (@panglesd, #1108)


# 2.4.0
Expand Down
21 changes: 16 additions & 5 deletions src/search/html.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,22 @@ type html = Html_types.div_content Tyxml.Html.elt
open Odoc_model
open Lang

let url id =
match
Odoc_document.Url.from_identifier ~stop_before:false
(id :> Odoc_model.Paths.Identifier.t)
with
let url { Entry.id; kind; doc = _ } =
let open Entry in
let stop_before =
(* Some module/module types/... might not have an expansion, so we need to
be careful and set [stop_before] to [true] for those kind of search
entries, to avoid linking to an inexistant page.

Docstring do not have an ID in the model, and use the ID from the parent
signature in search entries. Therefore, links to doc comments need
[stop_before] to be [false] to point to the page where they are present.

Values, types, ... are not sensitive to [stop_before], allowing us to
shorten the match. *)
match kind with Doc _ -> false | _ -> true
Copy link
Member

Choose a reason for hiding this comment

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

Hmm.

I think my point was that the intent of this line of code is to prevent a link potentially going to a module/module type/class/etc that doesn't have an expansion. I was mostly expecting to see something more like match kind with | Module _ -> true | ModuleType _ -> true ... | _ -> false - where the wildcard is matching the things like Exception and so on. I don't think it's terribly obvious that stop_before has no impact on those things, so instead we can just match Doc and let stop_before do nothing for the other things.

in
match Odoc_document.Url.from_identifier ~stop_before id with
| Ok url ->
let config =
Odoc_html.Config.v ~search_result:true ~semantic_uris:false
Expand Down
4 changes: 1 addition & 3 deletions src/search/html.mli
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ type html = Html_types.div_content Tyxml.Html.elt

val of_entry : Entry.t -> html list

val url :
Odoc_model.Paths.Identifier.Any.t ->
(string, Odoc_document.Url.Error.t) Result.result
val url : Entry.t -> (string, Odoc_document.Url.Error.t) Result.result

(** The below is intended for search engine that do not use the Json output but
Odoc as a library. Most search engine will use their own representation
Expand Down
4 changes: 2 additions & 2 deletions src/search/json_index/json_display.ml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
open Odoc_search

let of_entry { Entry.id; doc = _; kind = _ } h =
match Html.url id with
let of_entry entry h =
match Html.url entry with
| Result.Ok url ->
let html =
h
Expand Down
2 changes: 1 addition & 1 deletion test/search/dune
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
(cram
(deps %{bin:odoc} %{bin:odoc_print})
(enabled_if
(>= %{ocaml_version} 4.04.0)))
(>= %{ocaml_version} 4.08.0)))
6 changes: 3 additions & 3 deletions test/search/html_search.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ The index file, one entry per line:
{"id":[{"kind":"Page","name":"page"},{"kind":"Label","name":"a-title"}],"doc":"A title","kind":{"kind":"Doc","subkind":"Heading"},"display":{"url":"page/index.html#a-title","html":"<code class=\"entry-kind\">doc</code><code class=\"entry-title\"><span class=\"prefix-name\">page.</span><span class=\"entry-name\">a-title</span></code><div class=\"entry-comment\"><div><p>A title</p></div></div>"}}
{"id":[{"kind":"Root","name":"J"}],"doc":"a paragraph one","kind":{"kind":"Module"},"display":{"url":"page/J/index.html","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"entry-name\">J</span></code><div class=\"entry-comment\"><div><p>a paragraph one</p></div></div>"}}
{"id":[{"kind":"Root","name":"Main"}],"doc":"","kind":{"kind":"Module"},"display":{"url":"page/Main/index.html","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"entry-name\">Main</span></code><div class=\"entry-comment\"><div></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Module","name":"I"}],"doc":"","kind":{"kind":"Module"},"display":{"url":"page/Main/I/index.html","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">I</span></code><div class=\"entry-comment\"><div></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Module","name":"M"}],"doc":"","kind":{"kind":"Module"},"display":{"url":"page/Main/M/index.html","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">M</span></code><div class=\"entry-comment\"><div></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Module","name":"X"}],"doc":"","kind":{"kind":"Module"},"display":{"url":"page/Main/X/index.html","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">X</span></code><div class=\"entry-comment\"><div></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Module","name":"I"}],"doc":"","kind":{"kind":"Module"},"display":{"url":"page/Main/index.html#module-I","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">I</span></code><div class=\"entry-comment\"><div></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Module","name":"M"}],"doc":"","kind":{"kind":"Module"},"display":{"url":"page/Main/index.html#module-M","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">M</span></code><div class=\"entry-comment\"><div></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Module","name":"X"}],"doc":"","kind":{"kind":"Module"},"display":{"url":"page/Main/index.html#module-X","html":"<code class=\"entry-kind\">mod</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">X</span></code><div class=\"entry-comment\"><div></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Type","name":"t"}],"doc":"A comment","kind":{"kind":"TypeDecl","private":false,"manifest":"int","constraints":[]},"display":{"url":"page/Main/index.html#type-t","html":"<code class=\"entry-kind\">type</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">t</span><code class=\"entry-rhs\"> = int</code></code><div class=\"entry-comment\"><div><p>A comment</p></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Type","name":"tdzdz"}],"doc":"A comment aaaaaaaaaa","kind":{"kind":"TypeDecl","private":false,"manifest":null,"constraints":[]},"display":{"url":"page/Main/index.html#type-tdzdz","html":"<code class=\"entry-kind\">type</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.</span><span class=\"entry-name\">tdzdz</span><code class=\"entry-rhs\"> = A of int * int | B of int list * int</code></code><div class=\"entry-comment\"><div><p>A comment aaaaaaaaaa</p></div></div>"}}
{"id":[{"kind":"Root","name":"Main"},{"kind":"Module","name":"M"},{"kind":"Type","name":"t"}],"doc":"dsdsd","kind":{"kind":"TypeDecl","private":false,"manifest":null,"constraints":[]},"display":{"url":"page/Main/M/index.html#type-t","html":"<code class=\"entry-kind\">type</code><code class=\"entry-title\"><span class=\"prefix-name\">Main.M.</span><span class=\"entry-name\">t</span></code><div class=\"entry-comment\"><div><p>dsdsd</p></div></div>"}}
Expand Down
19 changes: 19 additions & 0 deletions test/search/module_aliases.t/main.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module X = struct
let x = 1

(** some comment *)
end

module Y = X
module Z = Y

module L = Stdlib.List

module type X = sig
val x : int
end

module type Y = X
module type Z = Y

module type L = module type of Stdlib.List
26 changes: 26 additions & 0 deletions test/search/module_aliases.t/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
Compile the files

$ ocamlc -c main.ml -bin-annot -I .

Compile and link the documentation

$ odoc compile main.cmt
$ odoc link main.odoc
$ odoc compile-index main.odocl

Search results only redirect to their definition point (not the
expansions). Comments link to the expansion they are in.

$ cat index.json | jq -r '.[] | "\(.id[-1].name) -> \(.display.url)"'
Main -> Main/index.html
X -> Main/index.html#module-X
x -> Main/X/index.html#val-x
X -> Main/X/index.html
Y -> Main/index.html#module-Y
Z -> Main/index.html#module-Z
L -> Main/index.html#module-L
X -> Main/index.html#module-type-X
x -> Main/module-type-X/index.html#val-x
Y -> Main/index.html#module-type-Y
Z -> Main/index.html#module-type-Z
L -> Main/index.html#module-type-L
Loading