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

Fix references to extension declarations #949

Merged
merged 6 commits into from
Oct 26, 2023
Merged

Fix references to extension declarations #949

merged 6 commits into from
Oct 26, 2023

Conversation

gpetiot
Copy link
Contributor

@gpetiot gpetiot commented Mar 31, 2023

Fix #932

Not working yet, I've written most of the boring code, some tweaking is needed to make it work.

  • the ExtensionDecl reference is added in Env
  • I think the lookup code in Ref_tools is correct
  • what is found in the Env is a Extension anchor, despite adding an ExtensionDecl

I think it's due to using Extension.t in the extension_decl type in Component:

type extension_decl =
  [ `ExtensionDecl of Identifier.Extension.t * Extension.Constructor.t ]

but changing everything creates too many changes. On the opposite side, it's probably possible to refactor the types, as maybe the existing Extension type might be enough in a lot of places, but I'm not familiar enough with the code to tell.

Ideas are welcome!

@gpetiot gpetiot marked this pull request as draft March 31, 2023 17:12
@gpetiot gpetiot changed the title [WIP] Fix references to extension declarations Fix references to extension declarations Mar 31, 2023
@gpetiot gpetiot marked this pull request as ready for review August 29, 2023 12:21
Copy link
Collaborator

@panglesd panglesd left a comment

Choose a reason for hiding this comment

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

I wonder how references should render: should we make a distinction between extension-A and extension-decl-A?

src/model/paths_types.ml Outdated Show resolved Hide resolved
src/model/paths_types.ml Outdated Show resolved Hide resolved
src/xref2/env.ml Outdated Show resolved Hide resolved
src/xref2/find.ml Outdated Show resolved Hide resolved
@gpetiot
Copy link
Contributor Author

gpetiot commented Aug 29, 2023

@panglesd the last commit (7bf16bd) does not compile, I tried to return a "nice" identifier in Ref_tools.ED.in_env that would contain the info of what is used for the url / what is displayed. It's hard to see if I went too far with the type changes (should Paths_types.Reference.any be modified?).
Le me know what you think. Feel free to take over and squash, rewrite, etc.

@gpetiot
Copy link
Contributor Author

gpetiot commented Sep 29, 2023

@Julow @jonludlam this one is ready to be reviewed :) (I don't have the rights to add reviewers myself)

Identifier.Extension.t * Extension.Constructor.t * Extension.t ]

type extension_decl =
[ `ExtensionDecl of Identifier.Extension.t * Extension.Constructor.t ]
Copy link
Collaborator

@EmileTrotignon EmileTrotignon Oct 3, 2023

Choose a reason for hiding this comment

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

I know its not the case in the rest of the codebase, but I think these require comments to explain what each field corresponds to. There is such a comment in paths.mli, which is very nice.
For me, the comments in paths.mli helps me guess what the fields are for, but future people trying to find their way around odoc might read this file first.

@EmileTrotignon
Copy link
Collaborator

I have left a comment on a spot I think more comments are warranted, apart from that I think the PR is good.

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.

I think the cleanup and comment commits could be squashed.

Otherwise, looks good to merge :)

@gpetiot
Copy link
Contributor Author

gpetiot commented Oct 10, 2023

I cleaned up and squashed the commits.

src/model/paths_types.ml Outdated Show resolved Hide resolved
@panglesd
Copy link
Collaborator

panglesd commented Oct 26, 2023

Made a last minor modification, added changelog and documentation. Approved by @Julow and disapproved by no-one, merging as soon as CI is green.

@panglesd panglesd merged commit 2fcaef1 into ocaml:master Oct 26, 2023
4 of 5 checks passed
panglesd added a commit that referenced this pull request Oct 26, 2023
Signed-off-by: Paul-Elliot <[email protected]>
@gpetiot gpetiot deleted the fix-932 branch October 26, 2023 23:57
Julow added a commit to Julow/opam-repository that referenced this pull request Dec 11, 2023
CHANGES:

### Added

- Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972)
  This includes the generation of an index and the display of the results in
  the UI (HTML only).

- Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019)
- Allow to omit parent type in constructor reference (@panglesd,
  @EmileTrotignon, ocaml/odoc#933)

### Fixed

- Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046)
- Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971)
- Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949)

### Changed

- Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045)
- Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028)
- Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967)
- Style: Sidebar is now stuck to the left of the content instead of the left of
  the viewport (@EmileTrotignon, ocaml/odoc#999)
Julow added a commit to Julow/opam-repository that referenced this pull request Dec 12, 2023
CHANGES:

### Added

- Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972)
  This includes the generation of an index and the display of the results in
  the UI (HTML only).

- Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019)
- Allow to omit parent type in constructor reference (@panglesd,
  @EmileTrotignon, ocaml/odoc#933)

### Fixed

- Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046)
- Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971)
- Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949)

### Changed

- Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045)
- Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028)
- Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967)
- Style: Sidebar is now stuck to the left of the content instead of the left of
  the viewport (@EmileTrotignon, ocaml/odoc#999)
Julow added a commit to Julow/opam-repository that referenced this pull request Dec 12, 2023
CHANGES:

### Added

- Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972)
  This includes the generation of an index and the display of the results in
  the UI (HTML only).

- Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019)
- Allow to omit parent type in constructor reference (@panglesd,
  @EmileTrotignon, ocaml/odoc#933)

### Fixed

- Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046)
- Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971)
- Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949)

### Changed

- Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045)
- Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028)
- Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967)
- Style: Sidebar is now stuck to the left of the content instead of the left of
  the viewport (@EmileTrotignon, ocaml/odoc#999)
Julow added a commit to Julow/opam-repository that referenced this pull request Dec 12, 2023
CHANGES:

### Added

- Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972)
  This includes the generation of an index and the display of the results in
  the UI (HTML only).

- Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019)
- Allow to omit parent type in constructor reference (@panglesd,
  @EmileTrotignon, ocaml/odoc#933)

### Fixed

- Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046)
- Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971)
- Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949)

### Changed

- Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045)
- Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028)
- Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967)
- Style: Sidebar is now stuck to the left of the content instead of the left of
  the viewport (@EmileTrotignon, ocaml/odoc#999)
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
CHANGES:

### Added

- Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972)
  This includes the generation of an index and the display of the results in
  the UI (HTML only).

- Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019)
- Allow to omit parent type in constructor reference (@panglesd,
  @EmileTrotignon, ocaml/odoc#933)

### Fixed

- Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050)
- Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046)
- Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971)
- Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949)

### Changed

- Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045)
- Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028)
- Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967)
- Style: Sidebar is now stuck to the left of the content instead of the left of
  the viewport (@EmileTrotignon, ocaml/odoc#999)
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.

Missing reference syntax for extension declarations
4 participants