docerror! to fix #1756

[st--]

Resolves #1756: With strict=true, Documenter errors when encountering any issue (based on checking doc.internal.errors in Selectors.runner in src/Builder.jl), but it doesn't actually say "Error" - only "Warning" - on the issue itself, which makes it non-intuitive to figure out what's going on (e.g. search log for error).

The reason for this is that on any issue, a tag is appended to the doc.internal.errors array, e.g.

push!(doc.internal.errors, :cross_references)

but in most places, it explicitly just prints a warning, e.g. in src/CrossReferences.jl:

@warn "'$slug' is not unique in $(Utilities.locrepr(page.source))."

In src/DocChecks.jl, there was already a loglevel() function that based on the strictness settings returns Logging.Error or Logging.Warn, so the log message of those checks was already correct.

In this PR, I combined the push! to doc.internal.errors & the appropriate logging message in a new function docerror!, and changed the other files to use this function as well.

[fredrikekre]

A disadvantage with this strategy is that we lose source information (all log messages will look like they come from this new function).

[st--]

A disadvantage with this strategy is that we lose source information (all log messages will look like they come from this new function).

Ah, that's unfortunate. Could we fix it by turning it into a @docerror macro?

[fredrikekre]

Yea, but maybe it is easier to keep the

@logmsg loglevel(...) "message"

pattern as was already there in some places?

[st--]

I thought it was helpful to directly couple the push! to doc.internal.errors with the message, as this removes a lot of duplicated code (it also ensures that the tag is the same in push! and loglevel())

[st--]

How about

macro docerror(doc, tag, msg, exs...)
    quote
        let
            push!($(doc).internal.errors, $(tag))
            if is_strict($(doc).user.strict, $(tag))
                @error $(msg) $(exs...)
            else
                @warn $(msg) $(exs...)
            end
        end
    end
end

and then instead of

push!(doc.internal.errors, :cross_references)
@logmsg is_strict(doc.user.strict, :cross_references) "<long message>"

can simply write

@docerror doc :cross_references "<long message>"

[fredrikekre]

Sure, if that works!

[st--]

I think it does (I started converting it over); could you turn on the workflow for me ?

[st--]

it should all be ready for CI now

[st--]

Phew, now all the tests are passing (except for codecov/patch, but I don't think there's an easy way of fixing that, so I hope it's acceptable as is) :)

[fredrikekre]

Great, can you add a note in CHANGELOG.md? Just mention that the issue is fixed.

[st--]

done!

[fredrikekre]

Code coverage dropped to 50% from 85%. Looks like every function that uses your new macro is marked as missed, so probably something is strange here...

[st--]

Code coverage dropped to 50% from 85%. Looks like every function that uses your new macro is marked as missed, so probably something is strange here...

That seems like a build issue, not an issue with my code - if you look at previous builds, e.g. codecov/project on e0dab4c, it says 86.91% (+0.17%). And a lot of the now missed lines have nothing to do with my changes.

Could you re-run the build ? GitHub has been having some issues as of yesterday. Maybe that's why it didn't properly get counted by codecov. (I've seen some issues with codecov just randomly claiming massively dropped coverage before.)

[fredrikekre]

Ok, that seems to have worked. Strange.