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

Some minor fixes #1271

Merged
merged 5 commits into from
Jan 13, 2025
Merged

Some minor fixes #1271

merged 5 commits into from
Jan 13, 2025

Conversation

jonludlam
Copy link
Member

This is on top of #1270

@jonludlam jonludlam force-pushed the further-voodoo-tweaks-v2 branch from cc59c8a to 639c014 Compare January 9, 2025 13:18
@jonludlam jonludlam added the no changelog This pull request does not need a changelog entry label Jan 9, 2025
This commit changes a couple of 'hard errors' -- assert failures --
into something that still not correct, but at least outputs something.
The problem here is hidden when the odoc and odocl directories are the same,
which is the default in odoc_driver.
@jonludlam jonludlam force-pushed the further-voodoo-tweaks-v2 branch from 639c014 to f6e8fe0 Compare January 13, 2025 15:44
src/markdown/odoc_md.ml Outdated Show resolved Hide resolved
{ Loc.line; column }
with _ ->
(* Presumably this is the above-mentioned bug that's being hit. *)
{ Loc.line = -1; column = -1 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

The only thing that raises a Invalid_argument is the array access at line 68. I think this could be handled better. It's also possible that locator is not computed right by massage_comment as it's so complicated.

Copy link
Member Author

Choose a reason for hiding this comment

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

There's quite a lot we can do with the markdown code, but I don't want to spend too much time on it until I'm sure people want it and are using it.

src/markdown/odoc_md.ml Show resolved Hide resolved
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.

Perfect, let's merge !

@jonludlam jonludlam merged commit 84023d6 into ocaml:master Jan 13, 2025
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog This pull request does not need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants