feature: introduce [$ dune ocaml doc] by EmileTrotignon · Pull Request #7262 · ocaml/dune

EmileTrotignon · 2023-03-10T15:10:42Z

This is a feature from issue #6831

It adds a new command "dune ocaml doc" that builds the doc, and then open its index in a browser. If no browser can be found, it print the location of the index.

There are no tests for this feature, as I do not know of a way to automatically test it.

I also only tested this on my machine, and this does very different stuff on different systems, so I would like if someone with a windows or an osx machine would be able to test it.

Alizter · 2023-03-11T19:33:17Z

Can you setup a cram test showing the feature? It should be possible to mock a browser for xdg-open in the cram test.

EmileTrotignon · 2023-03-14T09:13:23Z

@Alizter I have not managed to mock a browser for xdg-open, so I justed mock xdg-open itself.

Alizter

I know this is still a draft, but some initial comments and suggestions.

bin/build_cmd.mli

Alizter · 2023-03-16T12:06:50Z

boot/libs.ml

You probably want to revert this change. This happens automatically I think on macos.

test/blackbox-tests/test-cases/doc-browser.t/run.t

Alizter · 2023-03-16T12:16:33Z

test/blackbox-tests/test-cases/doc-browser.t/dune-project

@@ -0,0 +1,4 @@
+(lang dune 3.6)


We can use lang 3.8 for this.

3.8 gives me an error, I bumped to 3.7. Maybe a rebase will fix this ?

bin/build_cmd.ml

rgrinberg · 2023-04-10T22:06:17Z

bin/build_cmd.ml

+        let open Option.O in
+        let path = Env_path.path Env.initial in
+        let* cmd_name, args =
+          match Ocaml_config.system doc_ctx.ocaml_config with


We now have a Platform module that you can use

Oh that's very nice, thanks !

rgrinberg · 2023-04-10T22:06:33Z

bin/build_cmd.ml

+            absolute_toplevel_index_path
+        ];
+
+      let url = Printf.sprintf {|file://%s|} absolute_toplevel_index_path in


Any reason why you don't use normal strings?

I guess I was worried of having to escape the //, but that's not needed, I'll change that.

rgrinberg · 2023-04-10T22:09:24Z

bin/build_cmd.ml

+  let term =
+    let+ common = Common.term in
+    let config = Common.init common in
+    let request (setup : Import.Main.build_system) =


Could you minimize request to only what's required to build the docs and run everything else (printing, opening the docs) separately?

No I do not think I reasonably could, because I need the setup variable to compute the path were the index of the doc is located. I could escape it with a ref, but I don't think that would be better.

EmileTrotignon · 2023-04-27T14:09:59Z

I have finally tested this on a mac, and it works ! I still have to test it on windows.
I think mac and linux support is enough to get this out of draft though.

bin/build_cmd.mli

Alizter · 2023-05-08T20:50:42Z

bin/build_cmd.ml

+      | None ->
+        Dune_console.print
+          [ Pp.text
+              "No browser could be found, you will have to open the \


Please use Console.print instead.

Alizter · 2023-05-08T20:55:09Z

bin/build_cmd.ml

+            absolute_toplevel_index_path
+        ];
+
+      let url = Printf.sprintf "file://%s" absolute_toplevel_index_path in


String concatenation is perhaps simpler.

EmileTrotignon · 2023-05-11T09:03:04Z

@Alizter The last commit takes your comment into account.

Alizter

Just a few more comments.

Alizter · 2023-05-11T14:28:53Z

test/blackbox-tests/test-cases/doc-browser.t/run.t

+This tests shows how to use the `dune ocaml doc` command to open the
+documentation index to a browser.
+  $ export PATH=.:$PATH 
+  $ dune ocaml doc | sed -e 's/.*file:\/\/\([^ ]*\).*/\1/'


You can use a different deliminator rather than / for sed so you don't have to escape so many characters. Something like # should work.

Alizter · 2023-05-11T14:29:42Z

test/blackbox-tests/test-cases/doc-browser.t/open

@@ -0,0 +1,2 @@
+#!/bin/sh
+echo $@


Adding something like

echo "Web browser received arg:"

might make the test easier to read. Same for the other one.

Better yet have a single copy and create symlinks so we don't have to duplicate.

test/blackbox-tests/test-cases/doc-browser.t/dune-project

Alizter · 2023-05-11T14:31:28Z

bin/ocaml/doc.ml

+      Console.print
+        [ Pp.text
+            "No browser could be found, you will have to open the \
+             documentation yourself.\n"


Suggested change

documentation yourself.\n"

documentation yourself."

Alizter · 2023-05-11T14:34:22Z

bin/ocaml/doc.ml

+        (Path.to_absolute_filename cmd)
+        args ~env:Env.initial
+    | None ->
+      Console.print


If this is meant to be a warning, then you can use User_warning.emit instead.

Alizter · 2023-05-11T14:35:39Z

bin/ocaml/doc.ml

+        match Platform.OS.value with
+        | Platform.OS.Darwin -> Some ("open", [ "-u" ])
+        | Platform.OS.Linux -> Some ("xdg-open", [])
+        | Platform.OS.Windows -> Some ("start", [])
+        | Platform.OS.Other -> None


EmileTrotignon · 2023-07-10T09:34:30Z

Hey @Alizter
I have pushed a commit that takes your comment into account

Alizter · 2023-07-18T15:12:37Z

@emillon Could you take a look at this one?

emillon · 2023-07-18T15:50:45Z

bin/ocaml/doc.ml

+    let open Action_builder.O in
+    let+ () =
+      Alias.in_dir
+        ~name:(Dune_engine.Alias.Name.of_string "doc")


Can you add that as Dune_engine.Alias.doc? (and change the of_string "doc" in that file to use it)

There is already an Alias.doc variable, but it has the wrong type.

This has been moved since then, you can find it in the Alias0 module.

This is done. It feels a bit weird though, because it is the only such string in the alias module, except for "default".

You don't need to change the Alias module. This identifier already exists, it is in Alias0 module https://github.com/ocaml/dune/blob/main/src/dune_rules/alias0.ml

bin/ocaml/doc.ml

test/blackbox-tests/test-cases/doc-browser.t/dune-project

emillon · 2023-07-18T15:55:21Z

test/blackbox-tests/test-cases/doc-browser.t/xdg-open

@@ -0,0 +1,2 @@
+#!/bin/sh
+echo $@


there are a couple newlines to be fixed

bin/ocaml/doc.ml

emillon · 2023-07-20T12:20:27Z

@EmileTrotignon friendly ping. we're starting the 3.10 release process next week so it's a good opportunity to get this in, otherwise it won't be in before 3.11 (end of September probably)

Alizter · 2023-09-27T20:18:17Z

bin/ocaml/doc.ml

        | Linux -> Some ("xdg-open", [])
        | Windows -> None
-        | Other -> None
+        | Other | FreeBSD | NetBSD | OpenBSD -> None


FTR BSD platforms can have open or xdg-open but it's probably not worth adding support for it in this PR.

Co-authored-by: Jon Ludlam <jon@recoil.org>

Also windows, but should be tested.

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

@EmileTrotignon

CHANGES: - Introduce `$ dune ocaml doc` to open and browse documentation. (ocaml/dune#7262, fixes ocaml/dune#6831, @EmileTrotignon) - `dune cache trim` now accepts binary byte units: `KiB`, `MiB`, etc. (ocaml/dune#8618, @Alizter) - No longer force colors for OCaml 4.03 and 4.04 (ocaml/dune#8778, @rgrinberg) - Introduce new experimental odoc rules (ocaml/dune#8803, @jonjudlam) - Introduce the `runtest_alias` field to the `cram` stanza. This allows removing default `runtest` alias from tests. (@rgrinberg, ocaml/dune#8887) - Do not ignore libraries named `bigarray` when they are defined in conjunction with OCaml 5.0 (ocaml/dune#8902, fixes ocaml/dune#8901, @rgrinberg) - Dependencies in the copying sandbox are now writeable (ocaml/dune#8920, @rgrinberg) - Absent packages shouldn't prevent all rules from being loaded (ocaml/dune#8948, fixes ocaml/dune#8630, @rgrinberg) - Correctly determine the stanza of menhir modules when `(include_subdirs qualified)` is enabled (@rgrinberg, ocaml/dune#8949, fixes ocaml/dune#7610) - Display cache location in Dune log (ocaml/dune#8974, @nojb) - Re-run actions whenever `(expand_aliases_in_sandbox)` changes (ocaml/dune#8990, @rgrinberg) - Rules that only use internal dune actions (`write-file`, `echo`, etc.) can now be sandboxed. (ocaml/dune#9041, fixes ocaml/dune#8854, @rgrinberg) - Do not re-run rules when their location changes (ocaml/dune#9052, @rgrinberg) - Correctly ignore `bigarray` on recent version of OCaml (ocaml/dune#9076, @rgrinberg) - Add `test_` prefix to default test name in `dune init project` (ocaml/dune#9257, fixes ocaml/dune#9131, @9sako6) - Add `coqdoc_flags` field to `coq` field of `env` stanza allowing the setting of workspace-wide defaults for `coqdoc_flags`. (ocaml/dune#9280, fixes ocaml/dune#9139, @Alizter) - [coq rules] Be more tolerant when coqc --print-version / --config don't work properly, and fallback to a reasonable default. This fixes problems when building Coq projects with `(stdlib no)` and likely other cases. (ocaml/dune#8966, fix ocaml/dune#8958, @Alizter, reported by Lasse Blaauwbroek) - Dune will now run at a lower framerate of 15 fps rather than 60 when `INSIDE_EMACS`. (ocaml/dune#8812, @Alizter) - dune-build-info: when `version=""` is found in a `META` file, we now return `None` as a version string (ocaml/dune#9177, @emillon) - Dune can now be built and installed on Haiku (ocaml/dune#8795, fix ocaml/dune#8551, @Alizter) - Mark installed directories in `dune-package` files. This fixes `(package)` dependencies against packages that contain such directories. (ocaml/dune#8953, fixes ocaml/dune#8915, @emillon)

Alizter reviewed Mar 16, 2023

View reviewed changes

EmileTrotignon force-pushed the ocaml_doc_cmd branch from c3d7ee2 to 12d8932 Compare April 5, 2023 09:59

rgrinberg reviewed Apr 10, 2023

View reviewed changes

EmileTrotignon force-pushed the ocaml_doc_cmd branch from 9ffb13c to 00ade25 Compare April 27, 2023 14:02

EmileTrotignon marked this pull request as ready for review April 27, 2023 14:10

Alizter requested review from Alizter and rgrinberg May 8, 2023 15:03

Alizter reviewed May 8, 2023

View reviewed changes

Alizter reviewed May 11, 2023

View reviewed changes

emillon reviewed Jul 18, 2023

View reviewed changes

EmileTrotignon force-pushed the ocaml_doc_cmd branch 2 times, most recently from 860d879 to c637bbd Compare September 27, 2023 13:37

Alizter reviewed Sep 27, 2023

View reviewed changes

EmileTrotignon force-pushed the ocaml_doc_cmd branch from 5195917 to 4ba0aaf Compare October 11, 2023 14:48

EmileTrotignon and others added 6 commits October 16, 2023 15:06

prints the index path

0a03f7d

browser "progress"

2df3914

Co-authored-by: Jon Ludlam <jon@recoil.org>

works on my machine

213deb8

linux support

f77a5c5

Also windows, but should be tested.

add command

b3c106c

Add tests

4a05cb0

EmileTrotignon and others added 15 commits October 16, 2023 15:06

More idiomatic printing and better messages

393fbb1

Test improvement

b0cf22e

cleanup

2bbdbc3

test works on mac

046e963

Revert changes to libs

c9cb832

Code style update

5fa983c

Make windows situation clearer and little cleanup

f5181ab

Esacpe delim for sed

6967fb2

formatting

22895d0

Handle BSD systems

52d25f5

adds and use Alias.Name.doc

d457a92

fixes alias location

a495020

more explaination in test

4bccee8

add new system

3a9a4c1

_

d53fbe6

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg force-pushed the ocaml_doc_cmd branch from 4ba0aaf to d53fbe6 Compare October 16, 2023 21:16

rgrinberg added 4 commits October 16, 2023 15:22

_

6438393

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

fa846eb

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

a4d013a

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

_

1b30f32

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg linked an issue Oct 17, 2023 that may be closed by this pull request

dune doc #6831

Closed

rgrinberg added this to the 3.11 milestone Oct 17, 2023

rgrinberg changed the title ~~Ocaml doc cmd~~ feature: introduce [$ dune ocaml doc] Oct 17, 2023

rgrinberg merged commit dcce29f into ocaml:main Oct 17, 2023

rgrinberg added a commit that referenced this pull request Oct 17, 2023

chore: add CHANGES for #7262

094bd6d

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

rgrinberg added a commit that referenced this pull request Oct 17, 2023

chore: add CHANGES for #7262 (#8951)

aa50e9b

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>

emillon mentioned this pull request Nov 28, 2023

[new release] dune (15 packages) (3.12.0) ocaml/opam-repository#24862

Closed

		@@ -0,0 +1,2 @@
		#!/bin/sh
		echo $@ No newline at end of file

		@@ -0,0 +1,2 @@
		#!/bin/sh
		echo $@ No newline at end of file

Conversation

EmileTrotignon commented Mar 10, 2023

Uh oh!

Alizter commented Mar 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EmileTrotignon commented Mar 14, 2023

Uh oh!

Alizter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmileTrotignon commented Apr 27, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmileTrotignon commented May 11, 2023

Uh oh!

Alizter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmileTrotignon commented Jul 10, 2023

Uh oh!

Alizter commented Jul 18, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Alizter commented Mar 11, 2023 •

edited

Loading