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

Haddock: Fix very confusing formatting errors #2622

Merged
merged 1 commit into from
Jan 14, 2024
Merged

Conversation

DigitalBrains1
Copy link
Member

@DigitalBrains1 DigitalBrains1 commented Dec 12, 2023

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 4ac8481. 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:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files I didn't bother with this.

@leonschoorl
Copy link
Member

Here is another one:

-- done = exposeClockResetEnable (expectedOutput (topEntity <$> testInput)) clk rst

@leonschoorl
Copy link
Member

leonschoorl commented Dec 13, 2023

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
Copy link
Member Author

./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 DigitalBrains1 marked this pull request as draft December 13, 2023 16:56
@DigitalBrains1
Copy link
Member Author

DigitalBrains1 commented Dec 13, 2023

./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:

~SYM[2](~SYM[5]) <= ~TOBV[~ARG[9]][~TYP[9]];
end if;
~RESULT <= fromSLV(~SYM[2](~SYM[4]))

[/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 DigitalBrains1 changed the title Bundle doc: Fix very confusing formatting error Haddock: Fix very confusing formatting errors Dec 14, 2023
@DigitalBrains1 DigitalBrains1 marked this pull request as ready for review December 14, 2023 14:58
Copy link
Member

@leonschoorl leonschoorl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't run it through haddock, but all the edits look good

@@ -1187,9 +1187,9 @@ BlackBox:
begin
if ~IF~ACTIVEEDGE[Rising][0]~THENrising_edge~ELSEfalling_edge~FI(~ARG[3]) then
if ~ARG[7] ~IF ~ISACTIVEENABLE[4] ~THEN and ~ARG[4] ~ELSE ~FI then
~SYM[2](~SYM[5]) <= ~TOBV[~ARG[9]][~TYP[9]];
~SYM\[2](~SYM[5]) <= ~TOBV[~ARG[9]][~TYP[9]];
Copy link
Member

@leonschoorl leonschoorl Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of these very precise escapes in problem areas, have considered changing to the bird track style code blocks?

Ie change the whole block from:

@
line 1
...
line n
@

to:

> line 1
> ...
> line n

As I understand it those don't have any parsing for formatting/special characters.

You can't do that for some of the code blocks where you want to have embedded links for functions used. But when you don't need that, like in these blackboxes, it'll be more robust against future edits.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I 'unno. It's a large diff, it's busy work, I find the @...@ style more pleasant to look at, and it doesn't solve a fundamental issue. But if you feel strongly, I can change it.

BTW, I did check the whole source for more instances of this issue. And there aren't any more instances. Your grep correctly identified all of them.

Copy link
Member Author

@DigitalBrains1 DigitalBrains1 Dec 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it'll be more robust against future edits.

I feel somewhat defeated in this area. It feels like putting out a single twig in a forest fire. Haddock is just really hard to write and maintain.

I might write a vim syntax file to colour every special character (according to Haddock) in Haskell comments. That way I'll at least see directly that I need to watch out what I'm writing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel somewhat defeated in this area. It feels like putting out a single twig in a forest fire. Haddock is just really hard to write and maintain.

:(

Comment on lines -49 to +50
instance KnownDomain "System" where
type KnownConf "System" = 'DomainConfiguration "System" 10000 'Rising 'Asynchronous 'Defined 'ActiveHigh
instance KnownDomain 'System' where
type KnownConf 'System' = 'DomainConfiguration 'System' 10000 'Rising 'Asynchronous 'Defined 'ActiveHigh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what we want? It could either refer to the type synonym System or to the type-level string "System", I believe. They are the same thing. So it is either this or

instance KnownDomain \"System\" where
  type KnownConf \"System\" = 'DomainConfiguration \"System\" 10000 'Rising 'Asynchronous 'Defined 'ActiveHigh

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), 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.
@DigitalBrains1 DigitalBrains1 marked this pull request as ready for review January 9, 2024 16:40
Copy link
Member

@martijnbastiaan martijnbastiaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably ban @ completely. But that's probably hard to do without doctest like GHC API use..

@DigitalBrains1
Copy link
Member Author

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

@DigitalBrains1 DigitalBrains1 merged commit cb401b8 into master Jan 14, 2024
13 checks passed
@DigitalBrains1 DigitalBrains1 deleted the bundle-doc-fix branch January 14, 2024 17:09
mergify bot pushed a commit that referenced this pull request Jan 14, 2024
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), 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.

(cherry picked from commit cb401b8)
mergify bot pushed a commit that referenced this pull request Jan 14, 2024
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), 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.

(cherry picked from commit cb401b8)

# Conflicts:
#	clash-ffi/src/Clash/FFI/VPI/Object/Value/Parse.hs
#	clash-lib/src/Clash/Netlist/Util.hs
#	clash-lib/src/Clash/Primitives/DSL.hs
#	clash-prelude/src/Clash/Annotations/TopEntity.hs
#	clash-prelude/src/Clash/Explicit/Testbench.hs
#	clash-prelude/src/Clash/Intel/ClockGen.hs
#	clash-prelude/src/Clash/Tutorial.hs
#	clash-prelude/src/Clash/Xilinx/ClockGen.hs
DigitalBrains1 added a commit that referenced this pull request Jan 16, 2024
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), 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.

(cherry picked from commit cb401b8)

Co-authored-by: Peter Lebbing <[email protected]>
DigitalBrains1 added a commit that referenced this pull request Jan 17, 2024
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), 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.

(cherry picked from commit cb401b8)

Co-authored-by: Peter Lebbing <[email protected]>
hiddemoll pushed a commit that referenced this pull request Feb 1, 2024
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), 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants