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

Upgrade to Odoc 2.0.0 #333

Merged
merged 4 commits into from
Jul 13, 2021
Merged

Upgrade to Odoc 2.0.0 #333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61a932f
Upgrade to Odoc 2.0.0
Julow Jun 10, 2021
c0012a1
Simplify location handling
Julow Jul 8, 2021
699a6b4
Update changelog
Julow Jul 12, 2021
d8f0e6a
Sync dune generated opam file
NathanReb Jul 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#### Changed

- Use odoc-parser.0.9.0 (#333, @julow)

#### Deprecated

#### Fixed
Expand Down
2 changes: 1 addition & 1 deletion dune-project
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@
result
(ocaml-version
(>= 2.3.0))
(odoc (>= 1.4.1))
(odoc-parser (>= 0.9.0))
(lwt :with-test)
(alcotest :with-test)))
3 changes: 2 additions & 1 deletion lib/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
(preprocess
(action
(run %{bin:cppo} -V OCAML:%{ocaml_version} %{input-file})))
(libraries astring csexp fmt logs ocaml-version odoc.parser re result str))
(libraries astring csexp fmt logs ocaml-version odoc-parser re result str
compiler-libs.common))

(ocamllex lexer_mdx)

Expand Down
84 changes: 26 additions & 58 deletions lib/mli_parser.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
open! Compat

module Code_block = struct
type t = { location : Odoc_model.Location_.span; contents : string }
type t = { location : Odoc_parser.Loc.span; contents : string }
end

let drop_last lst =
Expand All @@ -14,8 +14,8 @@ let drop_first_and_last = function
| [] -> None
| first :: tl -> Some (first, drop_last tl)

let slice lines ~(start : Odoc_model.Location_.point)
~(end_ : Odoc_model.Location_.point) =
let slice lines ~(start : Odoc_parser.Loc.point) ~(end_ : Odoc_parser.Loc.point)
=
let lines_to_include =
Util.Array.slice lines ~from:(start.line - 1) ~to_:(end_.line - 1)
|> Array.to_list
Expand Down Expand Up @@ -46,62 +46,29 @@ let slice lines ~(start : Odoc_model.Location_.point)
let last_line = String.sub last_line 0 end_.column in
String.concat "\n" ([ first_line ] @ stripped @ [ last_line ])

(* Imagine a docstring that is within a file with four characters # of indentation. (I'll
use square brackets rather than parens to avoid escaping):

####[** foo
####
####bar
####
####baz *]
####val x : int
####val y : int

According to odoc, the "b" in "bar" is at column 0 inside the docstring and at column 4
within the broader file. That is correct. But it says the "f" in "foo" is at column 1
inside the docstring and column 5 within the file. This isn't right.

The problem is that it starts counting the inside-the-docstring column number from the
end of "[**", but doesn't add those three characters to the within-the-file column
number. Here, we make the adjustment.
*)
let account_for_docstring_open_token (location : Odoc_model.Location_.span) =
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 parser now keeps the location :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I talked too fast. This is still needed, an other force-push.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I debugged this a bit more and found that there is no bug into odoc-parser but rather that it expects the starting location to account for the comment opening. That's how odoc does it: https://github.com/ocaml/odoc/blob/master/src/loader/doc_attr.ml#L56
I pushed a fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, as described here: https://ocaml-doc.github.io/odoc-parser/Odoc_parser/index.html#val-parse_comment

let start_shift = 3 in
let end_shift = if location.start.line = location.end_.line then 3 else 0 in
{
location with
start = { location.start with column = location.start.column + start_shift };
end_ = { location.end_ with column = location.end_.column + end_shift };
}

let extract_code_blocks ~(location : Lexing.position) ~docstring =
let rec acc blocks =
List.map
(fun block ->
match Odoc_model.Location_.value block with
| `Code_block contents ->
let location =
if location.pos_lnum = block.location.start.line then
account_for_docstring_open_token block.location
else block.location
in
[ { Code_block.location; contents } ]
match Odoc_parser.Loc.value block with
| `Code_block (_metadata, { Odoc_parser.Loc.value = contents; _ }) ->
[ { Code_block.location = block.location; contents } ]
| `List (_, _, lists) -> List.map acc lists |> List.concat
| _ -> [])
blocks
|> List.concat
in
let parsed = Odoc_parser.parse_comment_raw ~location ~text:docstring in
let parsed = Odoc_parser.parse_comment ~location ~text:docstring in
List.iter
(fun error -> failwith (Odoc_model.Error.to_string error))
parsed.warnings;
(fun error -> failwith (Odoc_parser.Warning.to_string error))
(Odoc_parser.warnings parsed);
List.map
(fun element ->
match Odoc_model.Location_.value element with
| #Odoc_parser.Ast.nestable_block_element as e ->
acc
[ { Odoc_model.Location_.location = element.location; value = e } ]
| `Tag tag -> (
match element with
| { Odoc_parser.Loc.value = #Odoc_parser.Ast.nestable_block_element; _ }
as e ->
acc [ e ]
| { value = `Tag tag; _ } -> (
match tag with
| `Deprecated blocks -> acc blocks
| `Param (_, blocks) -> acc blocks
Expand All @@ -110,8 +77,8 @@ let extract_code_blocks ~(location : Lexing.position) ~docstring =
| `See (_, _, blocks) -> acc blocks
| `Before (_, blocks) -> acc blocks
| _ -> [])
| `Heading _ -> [])
parsed.value
| { value = `Heading _; _ } -> [])
(Odoc_parser.ast parsed)
|> List.concat

let docstrings lexbuf =
Expand All @@ -128,10 +95,10 @@ let docstrings lexbuf =
in
loop [] |> List.rev

let convert_pos (p : Lexing.position) (pt : Odoc_model.Location_.point) =
let convert_pos (p : Lexing.position) (pt : Odoc_parser.Loc.point) =
{ p with pos_lnum = pt.line; pos_cnum = pt.column }

let convert_loc (loc : Location.t) (sp : Odoc_model.Location_.span) =
let convert_loc (loc : Location.t) (sp : Odoc_parser.Loc.span) =
let loc_start = convert_pos loc.loc_start sp.start in
let loc_end = convert_pos loc.loc_end sp.end_ in
{ loc with loc_start; loc_end }
Expand All @@ -140,12 +107,13 @@ let docstring_code_blocks str =
Lexer.handle_docstrings := true;
Lexer.init ();
List.map
(fun (docstring, (location : Location.t)) ->
let blocks =
extract_code_blocks ~location:location.loc_start ~docstring
(fun (docstring, (cmt_loc : Location.t)) ->
let location =
{ cmt_loc.loc_start with pos_cnum = cmt_loc.loc_start.pos_cnum + 3 }
in
let blocks = extract_code_blocks ~location ~docstring in
List.map
(fun (b : Code_block.t) -> (b, convert_loc location b.location))
(fun (b : Code_block.t) -> (b, convert_loc cmt_loc b.location))
blocks)
(docstrings (Lexing.from_string str))
|> List.concat
Expand All @@ -155,7 +123,7 @@ let parse_mli file_contents =
[Text] and [Block] parts by using the starts and ends of those blocks as
boundaries. *)
let code_blocks = docstring_code_blocks file_contents in
let cursor = ref { Odoc_model.Location_.line = 1; column = 0 } in
let cursor = ref { Odoc_parser.Loc.line = 1; column = 0 } in
let lines = String.split_on_char '\n' file_contents |> Array.of_list in
let tokens =
List.map
Expand Down Expand Up @@ -185,11 +153,11 @@ let parse_mli file_contents =
in
let eof =
{
Odoc_model.Location_.line = Array.length lines;
Odoc_parser.Loc.line = Array.length lines;
column = String.length lines.(Array.length lines - 1);
}
in
let eof_is_beyond_location (loc : Odoc_model.Location_.point) =
let eof_is_beyond_location (loc : Odoc_parser.Loc.point) =
eof.line > loc.line || (eof.line = loc.line && eof.column > loc.column)
in
if eof_is_beyond_location !cursor then
Expand Down
4 changes: 0 additions & 4 deletions lib/mli_parser.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
open! Compat

module Code_block : sig
type t = { location : Odoc_model.Location_.span; contents : string }
end

val parse_mli : string -> (Document.line list, [ `Msg of string ]) Result.result
(** Slice an mli file into its [Text] and [Block] parts. *)
3 changes: 2 additions & 1 deletion mdx.opam
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,10 @@ depends: [
"re" {>= "1.7.2"}
"result"
"ocaml-version" {>= "2.3.0"}
"odoc" {>= "1.4.1"}
"odoc-parser" {>= "0.9.0"}
"lwt" {with-test}
"alcotest" {with-test}
"odoc" {with-doc}
]
build: [
["dune" "subst"] {dev}
Expand Down