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.

Sign up for GitHub

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

Jump to bottom

Odoc 3: CLI for -L and -P #1132

Merged
merged 21 commits into from
Jun 5, 2024
Merged

Odoc 3: CLI for -L and -P #1132

merged 21 commits into from
Jun 5, 2024

Conversation

panglesd
Copy link
Collaborator

@panglesd panglesd commented May 23, 2024 •
edited
Loading

This includes:

  • The -P and -L CLI options,
  • Several checks:
    • that no directory is contained another,
    • that the output file is contained in a package (this is disabled in case no -P, -L is passed)
  • (Lazily) storing file paths for each package in a hashtable
  • Functions for finding files them by name, or by path.
  • Tests for the checks
  • A test for finding by name, and finding by absolute path, using a hack.

The next stage is to implement new reference syntax, and integrate those "find by name" and "find by path" functions into the resolution of references.

panglesd
panglesd commented May 23, 2024
View reviewed changes
src/odoc/resolver.ml Outdated Show resolved Hide resolved
@panglesd panglesd added the no changelog This pull request does not need a changelog entry label May 23, 2024
EmileTrotignon
EmileTrotignon reviewed May 29, 2024
View reviewed changes
src/odoc/resolver.mli
val create :
important_digests:bool ->
directories:Fs.Directory.t list ->
open_modules:string list ->
roots:roots option ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think docs should be updated to explain the roots options here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rereading this part, I noticed that some of my terminology might be wrong: for the -P it is the package name that is relevant, but for the -L it is the library name.

EmileTrotignon
EmileTrotignon reviewed May 29, 2024
View reviewed changes
src/driver/odoc.ml Outdated Show resolved Hide resolved
EmileTrotignon
EmileTrotignon reviewed May 29, 2024
View reviewed changes
Copy link
Collaborator

@EmileTrotignon EmileTrotignon 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 there is no test for -L, is there a reason ?

@EmileTrotignon EmileTrotignon self-assigned this Jun 3, 2024
panglesd and others added 21 commits June 5, 2024 09:31
@panglesd
Add -L and -P options
e547fe5 
And store the result in a "Named root" table. Many things are not implemented in
this commit: knowing which is the package of the current compilation unit,
tests, ...

Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Give a type to reference parsing tokens
fd98a59 
This is just a refactor of the reference.ml file, for readability.

Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Do not catch Invalid_argument
08cc1cd 
See https://ocaml.org/manual/5.2/core.html#ss:predef-exn :

> Exception raised by library functions to signal that the given arguments do
> not make sense. The string gives some information to the programmer. As a
> general rule, this exception should not be caught, it denotes a programming
> error and the code should be modified not to trigger it.

Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Reference parsing refactor
c280788 
Some function took option input without any need for it!

Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Adding very custom test
8e735f7 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
-P and -L: Adding a test that the path are given all disjoints
808afd3 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Detect package name of the current compilation unit from -P/-L
921e600 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
More consistent naming: page_pkgnames and lib_pkgnames
e8c51cf 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Named roots: test broken reference
458e888 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Compatibility and formatting
dfcab13 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Add hack testing for absolute references
91bd21b 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
compatibility
684ee7e 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Odoc driver: Add -L and -P options
e0b0cd2 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Review comment: typo in CLI man page
b042cf1 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Odoc driver: remove debug artefact
04491f7 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
Review comment on attached docstring for Resolver.create
4075205 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
More coherent notation for named roots
5c27a45 
Signed-off-by: Paul-Elliot <peada@free.fr>
@panglesd
-L and -P options: improve tests
ed7088e 
Signed-off-by: Paul-Elliot <peada@free.fr>
@EmileTrotignon @panglesd
doc improvement
ba8a9e6
@EmileTrotignon @panglesd
add -L error test
199dceb
@EmileTrotignon @panglesd
doc adjustement
c0766eb
@jonludlam jonludlam merged commit 1ced6f2 into ocaml:master Jun 5, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EmileTrotignon EmileTrotignon EmileTrotignon left review comments

Assignees

@EmileTrotignon EmileTrotignon

Labels
no changelog This pull request does not need a changelog entry
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@panglesd @EmileTrotignon @jonludlam