-
Notifications
You must be signed in to change notification settings - Fork 481
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
Improve warnings in fix_doctest #2602
base: master
Are you sure you want to change the base?
Conversation
@mortenpi does this seem sane? If so I'll go ahead and add tests and a changelog entry. |
@@ -490,21 +490,23 @@ function fix_doctest(result::Result, str, doc::Documenter.Document; prefix::Muta | |||
r = Regex(rcode) | |||
codeidx = findfirst(r, content) | |||
if codeidx === nothing | |||
@warn "could not find code block in source file" | |||
@warn "could not find code block in source file $filename" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it would be kinda helpful if one would see the code block. Any reason why I shouldn't just print it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would be fine. I think we do that for some other messages already.
# next look for the particular input string in the given code block | ||
# make a regex of the input that matches leading whitespace (for multiline input) | ||
composed = prefix.content * result.raw_input | ||
rinput = replace(Documenter.regex_escape(composed), "\\n" => "\\n\\h*") | ||
r = Regex(rinput) | ||
inputidx = findfirst(r, code) | ||
if inputidx === nothing | ||
@warn "could not find input line in code block" | ||
startline = count("\n", data) | ||
@warn "could not find input line in code block in $filename:$startline" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here the "current" code block which was found: any particular reason not to print it here? Would have been helpful to me in the past
I haven't tested this, but from the looks of it, LGTM. Any extra context like this in the error messages is good. It would be good to have a test case, yes. We have some tests here for the fixing here (https://github.com/JuliaDocs/Documenter.jl/tree/master/test/doctests/fix). Not sure how easy it would be to expand those though to somehow test the new things. Maybe a few simple |
Resolves #2601
TODO: