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

Support for silent blocks #421

Open
Lupus opened this issue Mar 27, 2023 · 8 comments · May be fixed by #442
Open

Support for silent blocks #421

Lupus opened this issue Mar 27, 2023 · 8 comments · May be fixed by #442

Comments

@Lupus
Copy link

Lupus commented Mar 27, 2023

It would be nice to have support for blocks that evaluate, but do not produce any markdown output.

I'm writing some tutorial for a library in mdx, and need to have a snippet of code of 20-30 lines to present to the reader. Writing it in toplevel block directly is not very convenient as it won't get automatically formatted. Making it a file block solves the problem of formatting, but the code is not evaluated in mdx context. My best approach so far it to load it into toplevel via #use directive, but it outputs a lot of signatures along the way. Once loaded, I can present separate sections of the file via file blocks and then show some toplevel evaluation of the constructs defined in the file. With silent block I could load it without extra noise in the markdown output.

@Leonidas-from-XIV
Copy link
Collaborator

I am a bit confused, given the idea of MDX is to essentially present a fixed point of evaluation (modulo non-deterministic blocks), in that case not rendering the block would make it disappear from the next run. That's also why MDX files can be promoted using dune, so documents using silent blocks would silently promote into documents missing these blocks altogether.

Also, if it doesn't produce any output, how would error reporting work, if the block never renders anything? So whether it is correct or not would not yield any feedback to the documentation writer.

@Lupus
Copy link
Author

Lupus commented Mar 27, 2023

Silent block changes the environment and if consecutive blocks depend on it, diff & promotion workflow will work as expected. So far I worked around this via summary/details markdown tags (see this gist) to collapse the output of #require and #use at the beginning of my markdown file and hide this from the reader.

@Leonidas-from-XIV
Copy link
Collaborator

How will it work? If the block is silent, it will be missing in the output, thus promotion will promote a version that doesn't feature it, thus breaking the document.

@Lupus
Copy link
Author

Lupus commented Mar 27, 2023

I currently have the following block at the start of my mdx (using 2 backticks to not mess the outer markdown):

``ocaml
# #require "ppx_yojson_conv";;
# #require "my-lib";;
# #use "test/Mdx_parts.ml";;
``

It does produce some output, as Mdx_parts.ml has some modules and functions defined, but I want to hide inferred signatures from the reader at this point.

Later on in my document I expand on some subject, and output a relevant piece of Mdx_parts.ml like this:

<!-- $MDX file=test/Mdx_parts.ml,part=myCodePart -->
``ocaml
``

This shows a piece of code to the reader, and after I presented this code, I want to show how one can use it with toplevel blocks.

Probably I could just move that Mdx_parts.ml into a separate library that I can #require from toplevel block, this will avoid printing of module signatures when I don't want that. Library with example code for mdx sounds a bit counterintuitive at first though.

Both Rmarkdown [1] and IPython [2] do support supressing block output, so probably this feature is considered useful for side effects of some computation.

[1] https://bookdown.org/yihui/rmarkdown-cookbook/hide-all.html
[2] https://ipython.org/ipython-doc/3/interactive/tips.html#suppress-output

@Julow
Copy link
Collaborator

Julow commented Mar 31, 2023

Mdx already supports this kind of block, simply remove the prefixes #:

``ocaml
(* Comment to prevent Mdx from interpreting the '#'. *)
#require "ppx_yojson_conv";;
#require "my-lib";;
#use "test/Mdx_parts.ml";;
``

@Lupus
Copy link
Author

Lupus commented Mar 31, 2023

@Julow but I want the block to be actually executed, just the output to not be emitted in the document.

@Julow
Copy link
Collaborator

Julow commented Mar 31, 2023

It is:

```ocaml
let x = 42
```

```ocaml
# x ;;
- : int = 42
```

@Lupus
Copy link
Author

Lupus commented Mar 31, 2023

Oh, nice! Probably it's worth mentioning in the documentation explicitly that one can add a silent block that still evaluates.

@sorawee sorawee linked a pull request Dec 2, 2023 that will close this issue
sorawee added a commit to sorawee/mdx that referenced this issue Dec 2, 2023
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.

```
...
...
```
sorawee added a commit to sorawee/mdx that referenced this issue Dec 2, 2023
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.

    realworldocaml#1+1;;
    ...
    realworldocaml#2+2;;
    ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants