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

Revert to outputing a content-empty file when rendering a hidden module #1069

Merged
merged 4 commits into from
Jan 17, 2024

Conversation

panglesd
Copy link
Collaborator

@panglesd panglesd commented Jan 11, 2024

Hidden modules stopped creating an output file with "render source code", since calling html-generate on them was used to generate the associated source code.

However, this breaks dune rules in some cases (see issue #1013), as dune always expect an output to the execution of the command. See #1013 for a case where it happens.

With the separation of implementation and interface pipelines (see #1067), we will stop generating those content-empty files when rendering source code.

I think this fix should be included as a hotfix to the 2.3 and 2.4 branches, to maintain compatibility with dune rules (and ensure eio can build its doc).

@talex5 could you confirm that this patch fixes your issue?

Hidden modules were not rendered, since calling html-generate on them was used
to generate the associated source code.

However, this breaks dune rules in some cases (see issue ocaml#1013), as dune always
expect an output to the command.

This fix puts back the old behaviour with regard to rendering hidden files. It
is related to the separation of implementation and interface pipelines (see

Signed-off-by: Paul-Elliot <[email protected]>
Signed-off-by: Paul-Elliot <[email protected]>
@talex5
Copy link
Contributor

talex5 commented Jan 11, 2024

It seems to be working - thanks!

$ opam pin odoc.dev https://github.com/panglesd/odoc.git#fix-1013
$ opam pin odoc-parser.dev https://github.com/panglesd/odoc.git#fix-1013
$ dune build @doc; echo $status
0

Copy link
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

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

Looks good!

What do you think of deprecating this behavior so it can be removed again later ?

@jonludlam
Copy link
Member

I'm not sure it's necessary. We'll certainly need a fix to dune for this issue - even with this PR merged dune will produce a completely empty file for modules with double underscores, which is of no use to anyone. Once that's fixed (and we've merged the PR changing how we do source rendering), nobody will ever be calling html-generate on hidden modules any more, so we might not need to revert this change. It doesn't seem a bad principle that calling html-generate will produce something - perhaps an html file saying "This module has been hidden". What do you think?

@panglesd
Copy link
Collaborator Author

I agree that having odoc html-generate always output something seems something we want, and that telling the reader why the documentation does not appear (as opposed to outputing an empty file) seems better. I'll add that to the PR.

@Julow
Copy link
Collaborator

Julow commented Jan 17, 2024

I agree, it make sense to always generate something and the driver can easily skip generating these. The new paragraph is a nice touch !

@panglesd panglesd merged commit f64598a into ocaml:master Jan 17, 2024
9 of 10 checks passed
panglesd added a commit that referenced this pull request Jan 17, 2024
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.

4 participants