From 3fd9e80d05ad881695a9d33ff575f37703ce5162 Mon Sep 17 00:00:00 2001
From: Sorawee Porncharoenwase <sorawee.pwase@gmail.com>
Date: Sat, 2 Dec 2023 01:28:04 -0800
Subject: [PATCH] fix: make directives usable in the `ocaml` mode

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 #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.

    #1+1;;
    ...
    #2+2;;
    ...
---
 CHANGES.md                                        |  1 +
 lib/block.ml                                      |  7 +++++--
 test/bin/mdx-test/expect/simple-mld/test-case.mld | 12 ++++++++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index f8952ea1..6f62f15f 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -3,6 +3,7 @@
 #### Added
 
 - Handle the error-blocks syntax (#439, @jonludlam, @gpetiot)
+- Fix directives not allowed in the `ocaml` mode.
 
 ### 2.3.1
 
diff --git a/lib/block.ml b/lib/block.ml
index aa972fe4..239ef4f0 100644
--- a/lib/block.ml
+++ b/lib/block.ml
@@ -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
@@ -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 ->
diff --git a/test/bin/mdx-test/expect/simple-mld/test-case.mld b/test/bin/mdx-test/expect/simple-mld/test-case.mld
index e3355704..cc4b5806 100644
--- a/test/bin/mdx-test/expect/simple-mld/test-case.mld
+++ b/test/bin/mdx-test/expect/simple-mld/test-case.mld
@@ -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>
+]}