Documenter.jl icon indicating copy to clipboard operation
Documenter.jl copied to clipboard

Floating point numbers in doc strings cause method error

Open samoconnor opened this issue 6 years ago • 8 comments

Somehow the dollar amounts in the docstring below get parsed as floats and lead to a method error for mdflatten (and mdconvert).

"""
## `BonusAmount::String` -- *Required*
The Bonus amount is a US Dollar amount specified using a string (for example, "5"
represents \$5.00 USD and "101.42" represents \$101.42 USD). Do not include
currency symbols or currency codes.

"""

This patch fixes the problem, but perhaps the root-cause is in the parser?

diff --git a/src/Utilities/MDFlatten.jl b/src/Utilities/MDFlatten.jl
index a336ca2..0c1fa6e 100644
--- a/src/Utilities/MDFlatten.jl
+++ b/src/Utilities/MDFlatten.jl
@@ -64,6 +64,7 @@ function mdflatten(io, t::Table, parent)
 end

 # Inline nodes
+mdflatten(io, n::Number, parent) = print(io, n)
 mdflatten(io, text::AbstractString, parent) = print(io, text)
 mdflatten(io, link::Link, parent) = mdflatten(io, link.text, link)
 mdflatten(io, b::Bold, parent) = mdflatten(io, b.text, b)
diff --git a/src/Writers/HTMLWriter.jl b/src/Writers/HTMLWriter.jl
index 9a2881e..d92eec0 100644
--- a/src/Writers/HTMLWriter.jl
+++ b/src/Writers/HTMLWriter.jl
@@ -803,6 +803,8 @@ The `parent` argument is passed to allow for context-dependant conversions.
 """
 mdconvert(md; kwargs...) = mdconvert(md, md; kwargs...)

+mdconvert(n::Number, parent; kwargs...) = DOM.Node(string(n))
+
 mdconvert(text::AbstractString, parent; kwargs...) = DOM.Node(text)

 mdconvert(vec::Vector, parent; kwargs...) = [mdconvert(x, parent; kwargs...) for x in vec]

samoconnor avatar Jul 31 '17 07:07 samoconnor

This indeed comes from the parser. The dollar sign gets interpreted as a form of variable interpolation (not the usual one, but one than happens when the Markdown in docstrings gets parsed). There's a very brief note in the manual about this. The doc"" string macro should help get around this.

I don't think this warrants a change in Documenter, but maybe there should be updates to the parser to handle these cases a bit better.

mortenpi avatar Jul 31 '17 08:07 mortenpi

In my case, the documentation comes from here, and is converted to Julia doc strings by a code generator, so I can't easily change it.

I guess the thing to do is to use doc""" everywhere in the code generator.

One way or the other, Documenter.jl should deal with """ \$100.0 """ without crashing.

samoconnor avatar Jul 31 '17 08:07 samoconnor

It seems that doc""" does not solve the problem. The $ is still treated as a variable.

julia> VERSION
v"0.6.0"

julia> doc"""foo $bar"""
ERROR: UndefVarError: bar not defined

samoconnor avatar Jul 31 '17 09:07 samoconnor

Would you consider adding the following fallback methods to Documenter.jl as a workaround ?

mdflatten(io, n::Any, parent) = print(io, n)
mdconvert(n::Any, parent; kwargs...) = DOM.Node(string(n))

samoconnor avatar Jul 31 '17 09:07 samoconnor

bump

samoconnor avatar Sep 12 '17 23:09 samoconnor

As a user workaround you should be able to escape the dollar signs by sort of double escaping everything: """ \\\$100.0 """ actually.

To have the singly-escaped string working, it would have to be fixed in Base, since it's a parser issue. By the time the parsed Markdown reaches Documenter, the fact that there was a dollar sign somewhere is already lost, so it can't really be reliably worked around.

Not sure what the fix in Base should be -- with the dollar sign having a bunch of different meanings (usual variable interpolation, docstring interpolation, LaTeX syntax), it's a bit tricky. Nor am I sure whether it should be considered to be an bug at all.

Regarding the fallbacks -- it is true that currently you can end up with arbitrary objects in the parsed MD tree, and those will crash Documenter. But I think this is the best behaviour really, since Documenter does not know how to handle them properly anyway and silently hiding this is not very user-friendly. E.g. in this instance, with the fallback methods you would end up with numbers without dollar-signs in front, which would be a weird bug to track down. Instead it's better to let the user know that they should be doing something differently.

mortenpi avatar Sep 13 '17 09:09 mortenpi

Hello, I stumbled on this issue, as I had a $ sign in my documentation (I am working on an insurance related package), and it broke stuff, ie the package would not compile any more. I solved it with good old trial and error, and replaced $ with USD...

@mortenpi , do you think it might be worth mentioning in the docs? I could not find the reference you mention in your post on the 31 Jul 2017

I am happy to write a one-liner for the documentation, if deemed useful.

reumle avatar Aug 08 '22 23:08 reumle

There should be the note added in #739. But happy to take a PR if you think it can be improved.

mortenpi avatar Aug 08 '22 23:08 mortenpi