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.

Sign up for GitHub

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

Jump to bottom

Standard generators override custom generators #7108

Closed
vicuna opened this issue Dec 27, 2015 · 7 comments
Closed

Standard generators override custom generators #7108

vicuna opened this issue Dec 27, 2015 · 7 comments
Assignees
@gasche
Labels
bug ocamldoc

Comments

@vicuna
Copy link

vicuna commented Dec 27, 2015

Original bug ID: 7108
Reporter: @Armael
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2017-09-24T15:31:40Z)
Resolution: fixed
Priority: normal
Severity: tweak
Fixed in version: 4.03.0+dev / +beta1
Category: ocamldoc
Monitored by: @Armael

Bug description

This is a known (and documented) behavior: standard generators, loaded with e.g. the -html, -latex, ... options, override custom generators provided with option -g. This is a bit inconvenient, at least when using custom generators with ocamlbuild (see for example https://github.com/Armael/ocaml-libudev/blob/master/myocamlbuild.ml#L40 or https://github.com/ocsigen/tyxml/blob/master/myocamlbuild.ml#L57 for a workaround). gasche suggested (ocaml/ocamlbuild#10 (comment)) that the cleanest solution could be to change ocamldoc's behavior regarding this.

I could submit a PR where custom generators would always be loaded after standard ones, if this sounds like a satisfying solution.

File attachments
@vicuna
Copy link
Author

vicuna commented Dec 27, 2015

Comment author: @gasche

More precisely, I think that when a custom generator is defined by extending the html generator, as in the documentation example
http://caml.inria.fr/pub/docs/manual-ocaml/ocamldoc.html#sec333
, then passing the -html option should be valid and select the extended generator (and likewise when extending another generator and passing the corresponding option).

Having a quick look at the codebase, it looks like this would be rather natural to do: in the code of the -html, -latex etc. command-line options

ocaml/ocamldoc/odoc_args.ml

Lines 266 to 281 in 031cffd

(* generators *)
"-html", Arg.Unit (fun () -> set_generator
(Odoc_gen.Html (module Odoc_html.Generator : Odoc_html.Html_generator))),
M.generate_html ;
"-latex", Arg.Unit (fun () -> set_generator
(Odoc_gen.Latex (module Odoc_latex.Generator : Odoc_latex.Latex_generator))),
M.generate_latex ;
"-texi", Arg.Unit (fun () -> set_generator
(Odoc_gen.Texi (module Odoc_texi.Generator : Odoc_texi.Texi_generator))),
M.generate_texinfo ;
"-man", Arg.Unit (fun () -> set_generator
(Odoc_gen.Man (module Odoc_man.Generator : Odoc_man.Man_generator))),
M.generate_man ;
"-dot", Arg.Unit (fun () -> set_generator
(Odoc_gen.Dot (module Odoc_dot.Generator : Odoc_dot.Dot_generator))),
M.generate_dot ;

instead of calling set_generator with the base generators, use the get_foo_generator functions of

ocaml/ocamldoc/odoc_args.ml

Lines 19 to 51 in 031cffd

let get_html_generator () =
match !current_generator with
None -> (module Odoc_html.Generator : Odoc_html.Html_generator)
| Some (Odoc_gen.Html m) -> m
| Some _ -> failwith (M.current_generator_is_not "html")
;;
let get_latex_generator () =
match !current_generator with
None -> (module Odoc_latex.Generator : Odoc_latex.Latex_generator)
| Some (Odoc_gen.Latex m) -> m
| Some _ -> failwith (M.current_generator_is_not "latex")
;;
let get_texi_generator () =
match !current_generator with
None -> (module Odoc_texi.Generator : Odoc_texi.Texi_generator)
| Some (Odoc_gen.Texi m) -> m
| Some _ -> failwith (M.current_generator_is_not "texi")
;;
let get_man_generator () =
match !current_generator with
None -> (module Odoc_man.Generator : Odoc_man.Man_generator)
| Some (Odoc_gen.Man m) -> m
| Some _ -> failwith (M.current_generator_is_not "man")
;;
let get_dot_generator () =
match !current_generator with
None -> (module Odoc_dot.Generator : Odoc_dot.Dot_generator)
| Some (Odoc_gen.Dot m) -> m
| Some _ -> failwith (M.current_generator_is_not "dot")

Of course this is only a suspicion, one would need someone motivated to implement and test the change and report on it.

@vicuna
Copy link
Author

vicuna commented Dec 28, 2015

Comment author: @Armael

I see. Would the attached patch implement what you describe?
(I did not use the get_foo_generator functions as the raise an exception when the current generator is not of the expected kind - in this case I guess we just want to overwrite it)

@vicuna
Copy link
Author

vicuna commented Dec 28, 2015

Comment author: @gasche

I think that would be fine. Your gentler change (than raising an exception) also has the advantage that, if people insist on the previous behaviour, they can simply replace "-html" by "-man -html" as a workaround.

Did you test it on previously failing cases, or in the context of ocamlbuild#10? If yes, and if it works, I'll merge it; but can you add a Changes entry?

@vicuna
Copy link
Author

vicuna commented Jan 1, 2016

Comment author: @gasche

Armal: Ping? If you can confirm that this patch fixes the previous ocamldoc+ocamlbuild issues, I would be happy to merge it in 4.03 -- but then we should not wait to long.

@vicuna
Copy link
Author

vicuna commented Jan 2, 2016

Comment author: @Armael

Sorry for the delay. I tested the patch in the context of ocamlbuild#10: more precisely, using this self-contained example: https://github.com/Armael/manual-ocamlbuild/tree/custom-generator/examples/07-ocamldoc-custom-generator .

Attached is a new patch (generators2.patch) with a Changes entry, and small changes to the manual to update the documentation.

@vicuna
Copy link
Author

vicuna commented Jan 2, 2016

Comment author: @Armael

Oh, and the patch seems to work.

@vicuna
Copy link
Author

vicuna commented Jan 2, 2016

Comment author: @gasche

Merged in trunk,
f60e1d7

Thanks!

@vicuna vicuna closed this as completed Sep 24, 2017
@vicuna vicuna added the bug label Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gasche gasche

Labels
bug ocamldoc
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gasche @vicuna