-
Notifications
You must be signed in to change notification settings - Fork 101
Fix reference urls to sections #676
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
Conversation
- The two references in N are wrong, both are pointing to '../index.html#B', should be '#B' and '../M/index.html#B'. - Label defined in top-comment can't be referenced. - Reference paths are correct.
It was using the wrong parent (which defines the containing page).
| match parent with | ||
| | #Path.source as parent -> mk ~kind:"section" parent str_name | ||
| | `CoreType _ -> Error (Unexpected_anchor "core_type label parent") | ||
| | `Type (gp, _) -> mk ~kind:"section" gp str_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Identifier.LabelParent.t should probably not contain types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about inlined records ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Label are section names, not fields. I don't expect a heading (sections are only defined by headings) attached to anything inside a type (classes are covered in the Path.source case).
| [{"`Resolved":{"`Label":[{"`Identifier":{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"M"]}},"B"]}},[]] | ||
| [{"`Resolved":{"`Identifier":{"`Label":[{"`Root":[{"`RootPage":"test"},"Test"]},"A"]}}},[{"`Word":"First"},"`Space",{"`Word":"label"}]] | ||
| [{"`Resolved":{"`Identifier":{"`Label":[{"`Root":[{"`RootPage":"test"},"Test"]},"B"]}}},[{"`Word":"Floating"},"`Space",{"`Word":"label"}]] | ||
| [{"`Dot":[{"`Root":["M","`TUnknown"]},"C"]},[]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an other bug, references to labels in top-comments don't resolve.
Before OCaml 4.07, top-comments weren't recognized by the loader,
causing the following diff. This is potentially a bug.
--- a/../../../default/test/xref2/labels.t/run.t
+++ b/test/xref2/labels.t/run.t.corrected
@@ -8,9 +8,8 @@ Some are not in order because the 'doc' field appears after the rest in the outp
$ odoc_print test.odocl | jq -c '.. | .["`Heading"]? | select(.) | .[1]["`Label"]'
[{"`Root":[{"`RootPage":"test"},"Test"]},"B"]
- [{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"M"]},"D"]
[{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"M"]},"B"]
- [{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"M"]},"C"]
+ [{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"M"]},"D"]
[{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"N"]},"B"]
[{"`Root":[{"`RootPage":"test"},"Test"]},"A"]
@@ -23,9 +22,9 @@ We expect resolved references and the heading text filled in.
[{"`Resolved":{"`Identifier":{"`Label":[{"`Root":[{"`RootPage":"test"},"Test"]},"A"]}}},[{"`Word":"First"},"`Space",{"`Word":"label"}]]
[{"`Resolved":{"`Identifier":{"`Label":[{"`Root":[{"`RootPage":"test"},"Test"]},"B"]}}},[{"`Word":"Floating"},"`Space",{"`Word":"label"}]]
[{"`Dot":[{"`Root":["M","`TUnknown"]},"C"]},[]]
- [{"`Resolved":{"`Label":[{"`Identifier":{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"M"]}},"D"]}},[]]
+ [{"`Dot":[{"`Root":["M","`TUnknown"]},"D"]},[]]
[{"`Resolved":{"`Label":[{"`Identifier":{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"M"]}},"B"]}},[]]
- [{"`Resolved":{"`Label":[{"`Identifier":{"`Module":[{"`Root":[{"`RootPage":"test"},"Test"]},"N"]}},"B"]}},[]]
+ [{"`Dot":[{"`Root":["N","`TUnknown"]},"B"]},[]]
$ odoc html-generate --indent -o html test.odocl
| (cram | ||
| (deps %{bin:odoc} %{bin:odoc_print} %{bin:compile}) | ||
| (enabled_if | ||
| (>= %{ocaml_version} 4.07.0)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before 4.07, labels in top-comments were ignored by the loader. This is probably a bug.
Fix #659
It was pointing to the wrong parent page.