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

Use heading text for references to heading even across pages #1116

Merged
merged 6 commits into from
May 22, 2024

Conversation

panglesd
Copy link
Collaborator

Fix #941 and #779.

| Some (`Label (_, lbl)) -> Some lbl.Component.Label.text
| None -> None
in
identifier ?text id
Copy link
Member

Choose a reason for hiding this comment

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

Did this last commit fix a bug?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The interface for Ref_tools.resolve_reference was changed to also return a paragraph option: the "paragraph"1 is a possible replacement text computed when resolving the reference. In the case of a title, this "paragraph" is the title.

However, before the last commit, this paragraph was not always output as Some by resolve_reference, so there was some code in Link which looked for the title content in the missing case.

In short, the last commit does not fix a bug, it just moves this lookup of title content from Link.comment_inline_element to Ref_tools.resolve_reference so that the lookup of the title content is centralized in one place.

Footnotes

  1. paragraph here is just a type alias for inline_element with_location list. Not an actual paragraph.

Signed-off-by: Paul-Elliot <peada@free.fr>
Signed-off-by: Paul-Elliot <peada@free.fr>
Signed-off-by: Paul-Elliot <peada@free.fr>
Signed-off-by: Paul-Elliot <peada@free.fr>
Signed-off-by: Paul-Elliot <peada@free.fr>
Signed-off-by: Paul-Elliot <peada@free.fr>
@jonludlam
Copy link
Member

This seems fine, as far as it goes. At some point I think it'd be good if resolve_reference returned the component of the thing it found rather than just some extracted text, more like Tools.resolve_module and the like. This would allow a tool to look up an element by reference and then show the docs for that element.

In any case, this is a step in the right direction and fixes some bugs, so in it goes. Thanks!

@jonludlam jonludlam merged commit 623b7a1 into ocaml:master May 22, 2024
13 checks passed
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.

Title splicing problems on master
2 participants