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

Odoc compatibility #46

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Conversation

panglesd
Copy link
Contributor

@panglesd panglesd commented Dec 9, 2024

This PR makes sherlodoc compatible with odoc's ocaml/odoc#1251 (which hopefully makes it to master soon 🤞)

In fact, in addition to all the sidebar work (that is obviously making breaking changes on sherlodoc's side) it turns out that changes seemingly unrelated to indexing also broke sherlodoc:

Some bugfixes also "broke" (or more precisely, "fixed") some of sherlodoc's tests.

It would be great to find a solution to have sherlodoc and odoc's interaction more stable!

(This supersedes #42)

"Fewer conversions between Names and strings"
"Overhaul of module-type-of and shadowing"
"Remove core types and exceptions from identifiers"
Several changes:
- Entries are now defined in the `odoc_index` library,
- Entries can have new kinds (pages, source, ...)
- Indexes have the form of "skeletons of entries", that can be folded.
- Indexes can be created by odoc with the `odoc compile-index` command, and
  then consumed by sherlodoc.

These changes come from:
- ocaml/odoc#1228
- ocaml/odoc#1232
- ocaml/odoc#1233
- ocaml/odoc#1244
- ocaml/odoc#1250
- ocaml/odoc#1251
This function was used only in sherlodoc, it makes sense to be here.
In particular, odoc 3 fixed a bug where "hidden" modules were still
indexed. This is why e.g. `Base.StringLabels` is removed from the tests
output.
Odoc 3 also fixed the way extension constructor are handled.

Before, a "type" would be added, with ID the first constructor, making things
rather strange from the user eg as in the (now fixed) example:
```
type Main.MyExtension
```
displayed for
```
type extensible_type += MyExtension
```

This is now fixed. A test for the definition of the extensible type is also
added.
@@ -105,10 +105,10 @@
115 val Base.List.ignore_m : 'a t -> unit t
116 val Base.List.drop : 'a t -> int -> 'a t
116 val Base.List.take : 'a t -> int -> 'a t
125 mod Base.ListLabels
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thats weird, why did this show up before ? Its not part of Base. Is the Stdlib included in Base ?

Choose a reason for hiding this comment

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

Yes - here. The diff is because of a bugfix in the indexing code where we were not correctly ignoring this include, which should have been hidden due to the 'doc stop comment' here

@EmileTrotignon
Copy link
Collaborator

I had two separate ideas to improve the interaction between sherlodoc and odoc :

  1. Merge sherlodoc into the odoc repo to be able to fix compatibility in the same PR that breaks it.
  2. Move typename.ml inside odoc, I think this is the function that breaks most often.

Option 1 is more work but will have better results, option 2 is easier but compatibility might still break from time to time.

I think option 2 should be done no matter what because its very easy, and afterwards option 1 can be done if its needed and there is time for it.

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.

3 participants