Skip to content

Commit

Permalink
fix: make directives usable in the ocaml mode
Browse files Browse the repository at this point in the history
Prior this PR, the following code fails:

```
{@ocaml ocaml[
  #require "astring";;
  let x = Astring.strf;;
]}
```

because MDX incorrectly infers that the code block is a toplevel
interaction (from the fact that the block starts with `#`),
resulting in an error:

    incomplete toplevel entry: unexpected character '#'.
    Did you forget a space after the '#' at the start of the line

It is possible to workaround this issue as suggested in realworldocaml#421 by adding a
comment as a first line.

```
{@ocaml ocaml[
  (* This works! *)
  #require "astring";;
  let x = Astring.strf;;
]}
```

but ideally the workaround should not be needed.

One may wonder why the inference is needed, since the above code block
is already specified to be in the `ocaml` mode.
The answer appears to be that we are expected to use the inference heuristics
for a light sanity check, as the existing tests
("invalid ocaml" and "invalid toplevel" in `test_block.ml`) require:

    "let x = 2;;" in the toplevel mode should error with
    invalid toplevel syntax in toplevel blocks

    "# let x = 2;;" in the ocaml mode should error with
    toplevel syntax is not allowed in OCaml blocks

As a result, this PR keeps the light sanity check intact, but
adjusts the inference heuristics to be more conservative.
A block is now considered a toplevel interaction when
it starts with `#` followed by a space.
This fixes the issue, making it possible to use directives.
As a bonus, directives will now also work even when
the mode is not specified at all. But one disadvantage is that
this kind of code will no longer be considered invalid.

```
...
...
```
  • Loading branch information
sorawee committed Dec 2, 2023
1 parent 3226c6e commit 88bff95
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#### Added

- Handle the error-blocks syntax (#439, @jonludlam, @gpetiot)
- Fix directives not allowed in the `ocaml` mode.

### 2.3.1

Expand Down
7 changes: 5 additions & 2 deletions lib/block.ml
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ let guess_ocaml_kind contents =
| h :: t ->
let h = String.trim h in
if h = "" then aux t
else if String.length h > 1 && h.[0] = '#' then `Toplevel
else if String.length h > 2 && h.[0] = '#' && h.[1] = ' ' then `Toplevel
else `Code
in
aux contents
Expand Down Expand Up @@ -383,7 +383,10 @@ let mk_ocaml ~loc ~config ~header ~contents ~errors =
let kind = "OCaml" in
match config with
| { file_inc = None; part = None; env; non_det; _ } -> (
(* TODO: why does this call guess_ocaml_kind when infer_block already did? *)
(* mk_ocaml can be invoked because an explicit "$MDX ocaml" is given.
In such case, we still want to make it an error if
a toplevel interaction is given (see the test "invalid ocaml"),
so we use guess_ocaml_kind here to do that. *)
match guess_ocaml_kind contents with
| `Code -> Ok (OCaml { env = Ocaml_env.mk env; non_det; errors; header })
| `Toplevel ->
Expand Down
12 changes: 12 additions & 0 deletions test/bin/mdx-test/expect/simple-mld/test-case.mld
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,15 @@ Line 1, characters 15-18:
Error: This expression has type string but an expression was expected of type
int
]err}]}


{@ocaml ocaml[
#require "astring";;
let x = Astring.strf;;
]}


{[
# x;;
- : ('a, Format.formatter, unit, string) format4 -> 'a = <fun>
]}

0 comments on commit 88bff95

Please sign in to comment.