Fix indentation of code blocks

[aantron]

This issue is about adjusting the parser slightly, to delete some leading whitespace from code blocks.

Problem

When the author of docs writes

(** Some text

    {[
      let foo = 10
      let () = print_endline (string_of_int foo)
    ]} *)

...the code block gets parsed as

"
      let foo = 10
      let () = print_endline (string_of_int foo)
"

...and rendered in the HTML with all that left whitespace, which shouldn't be there. This forces people to use the ugly workaround:

(** Some text

{[
let foo = 10
let () = print_endline (string_of_int foo)
]} *)

odoc should do this automatically instead. odoc should find a line with the least amount of left whitespace (in this case, both lines have 6 leading spaces), and delete that much leading whitespace from all lines in the code block, in this case resulting in:

"
let foo = 10
let () = print_endline (string_of_int foo)
"

...which will get displayed nicely.

This was originally reported in #132 by @bobbypriambodo.

Fixing it

The issue should be pretty easy to solve. odoc already postprocesses code blocks immediately after reading them from comments, to remove leading and trailing blank lines. We need to add one more postprocessing step to that code, to remove the leading whitespace.

The rest of this post is a guide on how to do that :)

1. (Setup) Clone the repo and install development dependencies:

   git clone https://github.com/ocaml/odoc.git
   cd odoc
   opam pin add --no-action odoc .
   opam install --deps-only odoc
   

2. (Sanity check) Run make test to check the setup, and make sure tests start out passing :)

3. (Coding) The existing steps for postprocessing code blocks are here:

   odoc/src/parser/lexer.mll

   Lines 256 to 259 in af59e44

   	| "{[" (code_block_text as c) "]}"
   	{ let c = trim_leading_blank_lines c in
   	let c = trim_trailing_blank_lines c in
   	emit input (`Code_block c) }

   and the functions being called are defined here.

   So, we basically just need to add a third function, that goes through all lines in the code block text, finds which one has the least leading whitespace, and then goes through the lines again and removes that much whitespace from each one (you are welcome to use any other method, this is just the obvious suggestion :)).

4. (Testing) The parser is pretty heavily tested. So, once you are calling this new function, some of the tests will start failing. This is a good thing :) The first test to fail should be this one:

   odoc/test/parser/test.ml

   Line 273 in af59e44

   t "leading-whitespace" "{[ foo]}";

   ...and the message will be something like

   -- code-block.008 [leading-whitespace.] Failed --
   in _build/_tests/code-block.008.output:
   
   {[ foo]}
   
   --- expect/code-block/leading-whitespace.txt    2018-04-15 14:42:33.137223100 -0500
   +++ _actual/code-block/leading-whitespace.txt   2018-04-18 11:35:06.134115800 -0500
   @@ -1 +1 @@
   -((output (ok (((f.ml (1 0) (1 8)) (code_block " foo"))))) (warnings ()))
   +((output (ok (((f.ml (1 0) (1 8)) (code_block foo))))) (warnings ()))
   
   To replace expected output with actual, run
   
   bash _build/default/test/parser/_actual/replace.sh

   The - line is what the tests expected the output to be. It's what the parser used to output (note the space before foo). But now, we improved the parser, so we want the test to expect our new, corrected output (the line with the +). To replace the expected output, run the command the test framework is suggesting:

   bash _build/default/test/parser/_actual/replace.sh
   

   Then, run make test again. A few more tests that have leading whitespace will also fail. Inspect the output, and if the new output is correct, replace the expected output using the same command.

   See CONTRIBUTING.md for details on the testing setup.

5. (Optional) Write a new test. For example, it could give the parser a multiline code block, with lines that have different indentation levels. The testing framework will fail on it, because there is not yet any expected output. But, it will tell you what your actual output was, and give you a command for creating the expected output from it :)

   If you'd like to test your code "live" against a real project, you can install the development odoc with

   make clean
   opam install odoc
   

   This will install it from your development repo, since we pinned it earlier.

6. (Finishing up) Commit the new code (in lexer.mll), any changes to the tests (test/parser/test.ml), and the new expected outputs (they will be in test/parser/expect/**), and send the odoc repo a PR :)

   You may also want to unpin odoc now:

   opam unpin odoc
   

7. Thank you!! 🎉

[bobbypriam]

Thanks for the helpful writeup, Anton! Since I'm the one reporting, I think I'd be able to tackle this over the weekend. If anyone wants to get it first, feel free to do so :)

[pmetzger]

Can this be closed now that #134 is in?

[aantron]

I was just testing on a "real-world" example and found one little snag, so not quite yet. Will comment in #134.

[trefis]

I'm wondering if we shouldn't do the same thing for verbatim blocks, any opinion?

[dbuenzli]

Most likely.