Skip to content

Commit

Permalink
Fix url opened by "dune ocaml doc" (ocaml#10897)
Browse files Browse the repository at this point in the history
Prior to this change, "dune ocaml doc" would attempt to open an
incorrect url in browsers: "file://_build/default/_doc/_html/index.html".
This is incorrect because it appears to be a path relative to the
project root, but browsers interpret "file://foo/some/path" as the
path "/some/path" on the host referred to by "foo". In my experiments
however, the "foo" component is ignored and the browser redirects to
"file:///some/path" instead. Indeed, when I run "dune ocaml doc" I end
up at "file:///default/_doc/_html/index.html", with the "_build"
component of the path being ignored, and the remainder of the path
treated as an absolute path (which obviously doesn't refer to the
correct file).

It seems like droppping the "file://" prefix of the url is enough to
get browers to accept the relative path and correctly navigate to
"file:///absolute/path/to/index.html". Alternatively we could fix the
logic for computing an absolute path to index.html and keep the
"file://" prefix of the url, but that seems more complicated. I've
updated a variable name from "absolute_toplevel_index_path" to
"relative_toplevel_index_path" to reflect that it is in fact a path
relative to the project root and not an absolute path.

On MacOS I found that without the "file://" component of the URL the
"-u" argument to the "open" command no longer works, but just running
"open path/to/index.html" opens a browser and navigates to the
absolute path to the file.

Signed-off-by: Stephen Sherratt <[email protected]>
  • Loading branch information
gridbugs authored and anmonteiro committed Nov 17, 2024
1 parent 5d328fd commit 241738f
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
9 changes: 4 additions & 5 deletions bin/ocaml/doc.ml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ let term =
Alias.in_dir ~name:Dune_rules.Alias.doc ~recursive:true ~contexts:setup.contexts dir
|> Alias.request
in
let absolute_toplevel_index_path =
let relative_toplevel_index_path =
let toplevel_index_path =
let is_default ctx = ctx |> Context.name |> Dune_engine.Context_name.is_default in
let doc_ctx = List.find_exn setup.contexts ~f:is_default in
Expand All @@ -32,12 +32,12 @@ let term =
Path.(toplevel_index_path |> build |> to_string_maybe_quoted)
in
Console.print
[ Pp.textf "Docs built. Index can be found here: %s" absolute_toplevel_index_path ];
[ Pp.textf "Docs built. Index can be found here: %s" relative_toplevel_index_path ];
match
let open Option.O in
let* cmd_name, args =
match Platform.OS.value with
| Darwin -> Some ("open", [ "-u" ])
| Darwin -> Some ("open", [])
| Other | FreeBSD | NetBSD | OpenBSD | Haiku | Linux -> Some ("xdg-open", [])
| Windows -> None
in
Expand All @@ -47,8 +47,7 @@ let term =
in
( open_command
, (* First element of argv is the name of the command. *)
let url = "file://" ^ absolute_toplevel_index_path in
(cmd_name :: args) @ [ url ] )
(cmd_name :: args) @ [ relative_toplevel_index_path ] )
with
| Some (cmd, args) ->
Proc.restore_cwd_and_execve (Path.to_absolute_filename cmd) args ~env:Env.initial
Expand Down
1 change: 1 addition & 0 deletions doc/changes/10897.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix the URL opened by the command `dune ocaml doc`. (#10897, @gridbugs)
4 changes: 2 additions & 2 deletions test/blackbox-tests/test-cases/odoc/doc-browser.t/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ This tests shows how to use the `dune ocaml doc` command to open the
documentation index to a browser.
$ if [ "$(uname)" = Darwin ]; then mv xdg-open open; fi
$ export PATH=.:$PATH
$ dune ocaml doc | sed -e 's|^-u *||'
$ dune ocaml doc
Docs built. Index can be found here: _build/default/_doc/_html/index.html
open command received args:
file://_build/default/_doc/_html/index.html
_build/default/_doc/_html/index.html

0 comments on commit 241738f

Please sign in to comment.