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

revise manpage sections #1531

Merged
merged 8 commits into from
May 17, 2024
Merged

revise manpage sections #1531

merged 8 commits into from
May 17, 2024

Conversation

hannesm
Copy link
Member

@hannesm hannesm commented May 9, 2024

as proposed in #1530

lib_runtime/mirage_runtime.ml Outdated Show resolved Hide resolved
lib_runtime/mirage_runtime.ml Outdated Show resolved Hide resolved
reynir added a commit to robur-coop/unipi that referenced this pull request May 9, 2024
@reynir
Copy link
Member

reynir commented May 9, 2024

I tested this with unipi (see robur-coop/unipi#29) and I had to pass ~docs:Mirage_runtime.s_args to Cmdliner.Arg.info (which can be done once by defining a local function let info ?(docs = Mirage_runtime.s_args) = Cmdliner.Arg.info). Otherwise by default arguments end up in the OPTIONS section near the bottom right before COMMON OPTIONS. The sections are not completely sorted in reverse alphabetic order somehow.

@hannesm
Copy link
Member Author

hannesm commented May 9, 2024

The reverse lexical ordering is for unknown sections -- where name, synopsis, description, commands, arguments, options, common options, exit codes, bugs, .. are known (and have a hardcoded ordering in cmdliner) -- and here the unknown ones are interjected in reverse lexical order after description.

@reynir
Copy link
Member

reynir commented May 9, 2024

I discovered we can control the order of the sections by specifying them in the desired order in the man page:

diff --git a/lib_runtime/functoria/functoria_runtime.ml b/lib_runtime/functoria/functoria_runtime.ml
index 1a162759..5b49f934 100644
--- a/lib_runtime/functoria/functoria_runtime.ml
+++ b/lib_runtime/functoria/functoria_runtime.ml
@@ -48,7 +48,7 @@ let initialized = ref false
 let help_version = 63
 let argument_error = 64
 
-let with_argv keys s argv =
+let with_argv sections keys s argv =
   let open Cmdliner in
   if !initialized then ()
   else
@@ -68,7 +68,8 @@ let with_argv keys s argv =
         Cmd.Exit.info ~doc:"on OCaml uncaught exception." 255;
       ]
     in
-    match Cmd.(eval_value ~argv (Cmd.v (info ~exits s) t)) with
+    let man = List.map (fun s -> `S s) sections in
+    match Cmd.(eval_value ~argv (Cmd.v (info ~man ~exits s) t)) with
     | Ok (`Ok _) ->
         initialized := true;
         ()
diff --git a/lib_runtime/functoria/functoria_runtime.mli b/lib_runtime/functoria/functoria_runtime.mli
index bbee324f..6ae7611a 100644
--- a/lib_runtime/functoria/functoria_runtime.mli
+++ b/lib_runtime/functoria/functoria_runtime.mli
@@ -41,7 +41,7 @@ val register : 'a Cmdliner.Term.t -> unit -> 'a
 
     [f] will raise [Invalid_argument] if called before cmdliner's evaluation. *)
 
-val with_argv : unit Cmdliner.Term.t list -> string -> string array -> unit
+val with_argv : string list -> unit Cmdliner.Term.t list -> string -> string array -> unit
 (** [with_argv keys name argv] evaluates the [keys] {{!Key.term} terms} on the
     command-line [argv]. [name] is the executable name. On evaluation error the
     application calls [exit(3)] with status [64]. If [`Help] or [`Version] were
diff --git a/lib_runtime/mirage_runtime.ml b/lib_runtime/mirage_runtime.ml
index a7f316ff..07736166 100644
--- a/lib_runtime/mirage_runtime.ml
+++ b/lib_runtime/mirage_runtime.ml
@@ -16,11 +16,11 @@
 
 open Cmdliner
 
-let s_ocaml = "COMMON OCAML RUNTIME OPTIONS"
+let s_ocaml = "OCAML RUNTIME OPTIONS"
 let s_log = "LOG AND MONITORING OPTIONS"
 let s_net = "NETWORK OPTIONS"
 let s_disk = "DISK OPTIONS"
-let s_arg = "UNIKERNEL ARGUMENTS"
+let s_arg = "ARGUMENTS"
 
 type log_threshold = [ `All | `Src of string ] * Logs.level option
 
@@ -249,7 +249,7 @@ let run_leave_iter_hooks () = run leave_iter_hooks
 let at_exit f = add f exit_hooks
 let at_leave_iter f = add f leave_iter_hooks
 let at_enter_iter f = add f enter_iter_hooks
-let with_argv = Functoria_runtime.with_argv
+let with_argv = Functoria_runtime.with_argv [ s_arg; s_net; s_log; s_disk; s_ocaml ]
 let runtime_args = Functoria_runtime.runtime_args
 let register = Functoria_runtime.register
 let argument_error = Functoria_runtime.argument_error

@hannesm
Copy link
Member Author

hannesm commented May 9, 2024

oh nice, @reynir -- would you mind to push that change to this PR?

@hannesm
Copy link
Member Author

hannesm commented May 9, 2024

-val with_argv : unit Cmdliner.Term.t list -> string -> string array -> unit
+val with_argv : string list -> unit Cmdliner.Term.t list -> string -> string array -> unit
 (** [with_argv keys name argv] evaluates the [keys] {{!Key.term} terms} on the

We should describe the new input (and make it an optional argument)!?

@reynir
Copy link
Member

reynir commented May 9, 2024

Please see latest commit. I didn't revert the change of section names, but maybe we should? I don't have any opinions on that.

@hannesm
Copy link
Member Author

hannesm commented May 9, 2024

So, people will forget to use s_arg (which actually we don't need to redefine, we can just use Cmdliner.Manpage.s_arguments).

I propose to make the sections ARGUMENTS OPTTIONS and then our mirage-custom ones. The reasoning is that people will forget, and should nevertheless see their stuff on the top. WDYT?

lib_runtime/mirage_runtime.ml Outdated Show resolved Hide resolved
lib_runtime/mirage_runtime.ml Outdated Show resolved Hide resolved
lib_runtime/mirage_runtime.ml Outdated Show resolved Hide resolved
lib_runtime/mirage_runtime.ml Outdated Show resolved Hide resolved
@hannesm
Copy link
Member Author

hannesm commented May 9, 2024

I tested this with the above linked unipi PR, and to me this is a great improvement.

Especially that the default section for optional arguments and required arguments (if you do not specify ~docs in your Cmdliner.Arg.info) are shown at the top in the manpage -- so users won't need to search for these.

Any other opinions?

hannesm and others added 8 commits May 9, 2024 21:02
Co-authored-by: Reynir Björnsson <reynir@reynir.dk>
The order of manpage sections can be enforced by passing a
Cmdliner.Manpage.t with the sections in order.
Co-authored-by: Hannes Mehnert <hannes@mehnert.org>
@hannesm hannesm mentioned this pull request May 16, 2024
14 tasks
@samoht
Copy link
Member

samoht commented May 17, 2024

Looks great, thanks!

@samoht samoht merged commit fb08454 into mirage:main May 17, 2024
3 checks passed
@hannesm hannesm deleted the manpage branch May 17, 2024 10:27
hannesm added a commit to hannesm/opam-repository that referenced this pull request May 17, 2024
CHANGES:

- BREAKING: remove `~name` parameter from Mirage.Runtime_args.create
  (mirage/mirage#1541 @samoht, fixes mirage/mirage#1532)
- BREAKING: remove `~name` parameter from Mirage.runtime_arg, and use a
  string (instead of a format string) as third parameter (mirage/mirage#1541 @samoht)
- constrain the `start` function to `unit Lwt.t`. Previously, there was no
  restrictions, and lots of time was spent in debugging when a unikernel
  resulted in `unit Lwt.t Lwt.t` (@Julow mirage/mirage#1524)
- revise man page sections and ordering: ARGUMENTS, OPTIONAL, NETWORK OPTIONS,
  DISK OPTIONS, LOG AND MONITORING OPTIONS, OCAML RUNTIME OPTIONS. Previously,
  the ARGUMENTS and OPTIONS were put later, and were hard to find. These are
  the sections where unikernel-specific arguments are put by default
  (mirage/mirage#1531 @hannesm @reynir)
- add --net=host and --net=ocaml to reduce confusion. --net=host uses the
  TCP/IP socket stack, --net=ocaml the OCaml network stack (mirage/mirage#1525 @hannesm)
- quote Runtime_arg.call (mirage/mirage#1522 @Julow)
- documentation fixes (inline examples @Julow mirage/mirage#1523, @hannesm mirage/mirage#1537 (fixes
  mirage/mirage#1512 reported by @reynir), Runtime_args.create mirage/mirage#1541 @samoht)
- fix the build instructions of the generated opam file: since 4.5.0
  `mirage build` is no longer available, use `make "build"` (mirage/mirage#1527 @hannesm)
- add RELEASE.md, a guide on how to cut a mirage release (mirage/mirage#1519 @samoht)
- allow git 3.16 (mirage/mirage#1536 @hannesm)
- use mirage-bootvar (using dune variant) instead of parse-argv and
  mirage-bootvar-xen, mirage-bootvar-solo5, mirage-bootvar-unix
  (mirage/mirage#1533 @hannesm)
- BUGFIX: reset the lexer location before applying functors in generated code
  (mirage/mirage#1539 @samoht, fixes mirage/mirage#1520 @hannesm)
- BUGFIX: fix off-by-one locations for mirage/main.ml (mirage/mirage#1540 @samoht, fixes
  mirage/mirage#1528 @hannesm)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants