clash-lang / clash-compiler

Haskell to VHDL/Verilog/SystemVerilog compiler
https://clash-lang.org/
Other
1.43k stars 151 forks source link

Haddock: Fix very confusing formatting errors #2622

Closed DigitalBrains1 closed 8 months ago

DigitalBrains1 commented 9 months ago

Because of mistakes escaping special characters, some documentation showed the operators $ and * where the Functor and Applicative operators were intended. Since they are all very common operators, this can be very confusing to the reader.

Additionally, it was discovered that later versions of Haddock start interpreting Markdown-style links in code blocks, breaking the rendering of the primitive blackboxes in Clash.Tutorial. This problem does not occur with GHC 9.0.2 (Haddock 2.25.1) GHC 8.10.7 (Haddock 2.24.2), which we use to build the documentation for Hackage. But it does occur with at least GHC 9.6.

To make such things less problematic, we decided to use bird tracks (lines starting with >) for code blocks that are not Haskell code, and only use the @...@ style of code block for Haskell code. Bird tracks don't use any formatting at all. All code blocks with something other than Haskell code have been converted to bird tracks. Additionally, some escaping issues in Haskell code blocks were fixed when they were noticed.

My intention is to upload new 1.8.1 documentation with this fix to Hackage. That's going to require a small amount of creativity, as I don't want to upload the changes of 4ac84816d. But that should work out just fine.

[edit] After this PR was merged, I discovered that I had somehow misinterpreted which version of GHC we use to build the documentation for Hackage. I thought it was 9.0.2, and that is what the commit message for this PR says. But we use an even older one, we build the documentation for Hackage with 8.10.7. This does not really change anything, though. [/edit]

Still TODO:

leonschoorl commented 9 months ago

Here is another one: https://github.com/clash-lang/clash-compiler/blob/930641c63517cedf59dff6754230e74cfe53b136/clash-prelude/src/Clash/Explicit/Testbench.hs#L452

leonschoorl commented 9 months ago

There are some more weird one:

[dist-newstyle/build/x86_64-linux/ghc-9.6.3/clash-prelude-1.9.0]$ rg 'href="[^"#]{0,9}"' .
./clash-prelude/Clash-Explicit-Testbench.html
62:    done           = exposeClockResetEnable (expectedOutput (topEntity <a href="$">$</a> testInput)) clk rst
                                                                             ^^^^^^^^
./clash-prelude/Clash-Tutorial.html
920:            ~SYM<a href="~SYM[5]">2</a> &lt;= ~TOBV[~ARG[9]][~TYP[9]];
                           ^^^^^^^^
922:          ~RESULT &lt;= fromSLV(~SYM<a href="~SYM[4]">2</a>)
                                                 ^^^^^^^^
933:            ~SYM<a href="~SYM[5]">2</a> &lt;= ~ARG[9];
                             ^^^^^^^^
935:          ~RESULT &lt;= ~SYM<a href="~SYM[4]">2</a>
                                         ^^^^^^^^

./clash-prelude/Clash-Prelude-Testbench.html
93:    done           = exposeClockResetEnable (expectedOutput (topEntity <a href="$">$</a> testInput)) clk rst
                                                                             ^^^^^^^^

./clash-prelude/Clash-Signal-Bundle.html
20:  bundle   (MkPair as bs) = MkPair <a href="$">$</a> as <a href="*">*</a> bs
                                                              ^^^^^^^^
23:  unbundle pairs = MkPair (getA <a href="$">$</a> pairs) (getB <a href="$">$</a> pairs)
                                                                     ^^^^^^^^

./clash-prelude/Clash-Sized-RTree.html
82:<a href="BLANKLINE">BLANKLINE</a>
            ^^^^^^^^

./clash-prelude/Clash-Explicit-Signal.html
383:  bundle   (MkPair as bs) = MkPair <a href="$">$</a> as <a href="*">*</a> bs
                                         ^^^^^^^^              ^^^^^^^
386:  unbundle pairs = MkPair (getA <a href="$">$</a> pairs) (getB <a href="$">$</a> pairs)
                                      ^^^^^^^^                        ^^^^^^^^

./clash-prelude/Clash-Signal.html
742:  bundle   (MkPair as bs) = MkPair <a href="$">$</a> as <a href="*">*</a> bs
                                         ^^^^^^^^              ^^^^^^^^

745:  unbundle pairs = MkPair (getA <a href="$">$</a> pairs) (getB <a href="$">$</a> pairs)
                                       ^^^^^^^^                       ^^^^^^^^

./clash-prelude/Clash-Prelude.html
402:  bundle   (MkPair as bs) = MkPair <a href="$">$</a> as <a href="*">*</a> bs
                                          ^^^^^^^^             ^^^^^^^^
405:  unbundle pairs = MkPair (getA <a href="$">$</a> pairs) (getB <a href="$">$</a> pairs)
                                       ^^^^^^^^                       ^^^^^^^^

./clash-prelude/Clash-HaskellPrelude.html
143:   <code>x = fromEnum n' - fromEnum n</code>, <code>c x = bool (&gt;=) (<a href="=)">(x</a> 0)</code>
                                                                               ^^^^^^^^
DigitalBrains1 commented 9 months ago
./clash-prelude/Clash-HaskellPrelude.html
143:   <code>x = fromEnum n' - fromEnum n</code>, <code>c x = bool (&gt;=) (<a href="=)">(x</a> 0)</code>
                                                                               ^^^^^^^^

That one is upstream. It renders the part <=) (x > in that line as a link.

DigitalBrains1 commented 9 months ago
./clash-prelude/Clash-Tutorial.html
920:            ~SYM<a href="~SYM[5]">2</a> &lt;= ~TOBV[~ARG[9]][~TYP[9]];
                           ^^^^^^^^
922:          ~RESULT &lt;= fromSLV(~SYM<a href="~SYM[4]">2</a>)
                                                 ^^^^^^^^
933:            ~SYM<a href="~SYM[5]">2</a> &lt;= ~ARG[9];
                             ^^^^^^^^
935:          ~RESULT &lt;= ~SYM<a href="~SYM[4]">2</a>
                                         ^^^^^^^^

[edit 2] Corresponding source code: https://github.com/clash-lang/clash-compiler/blob/930641c63517cedf59dff6754230e74cfe53b136/clash-prelude/src/Clash/Tutorial.hs#L1190-L1192 [/edit 2]

Oh no. That one does not occur when building with GHC 9.0.2, which we use for Hackage doc uploads. It does occur on GHC 9.6.3. I can fix it, that's not the problem. The problem is this: I've often said "the only way to know that your Haddock comes out right is to look at the result carefully and click all links". Apparently that's not enough. Now, when we upgrade to a new Haddock version, we're apparently supposed to carefully reread our entire generated documentation to see if Haddock hasn't changed its interpretation. Undoable.

I haven't checked Haddock's changelog to see if this is mentioned somewhere, but I'm not holding much hope. The changelog does not mention this change in interpretation.

[edit] Note that Haddock's documentation on special characters puts you on the wrong track here. The characters used in this problematic bit aren't listed. You're supposed to imply from the documentation on [Markdown style](links) that those characters can also be special.

We should probably not over-quote those Markdown-style links to prevent it looking like LaTeX. Currently, it's rendered correctly, but they might (by accident?) start rendering it differently in a future version. [/edit]

DigitalBrains1 commented 8 months ago

If you ban @ you can't link identifiers anymore in Haskell code blocks. That's baby and bathwater territory. My suggestion initially was "use bird tracks unless you wish to use formatting", but Christiaan quite decisively said "let's use @ ... @ blocks for Haskell code and bird tracks for other things".