Skip to content

Conversation

@ranocha
Copy link
Contributor

@ranocha ranocha commented Mar 15, 2022

As suggested by @KristofferC in JuliaDocs/Documenter.jl#1779 (comment). This is related to problems using CodeTracking.jl with Documenter.jl (see JuliaDocs/Documenter.jl#1779) or Jupyter notebooks (see #51). Instead of throwing an exception, code_string now returns nothing if definition returns nothing. This makes it easier to handle such a problematic case by the user.

Since the issue only appears in these on-standard environments, I do not really know a nice way to integrate an appropriate test in the current framework.

As suggested by @KristofferC in JuliaDocs/Documenter.jl#1779 (comment). This is related to problems using CodeTracking.jl with Documenter.jl (see JuliaDocs/Documenter.jl#1779) or Jupyter notebooks (see #51). Instead of throwing an exception, `code_string` now returns `nothing` if `definition` returns `nothing`. This makes it easier to handle such a problematic case by the user.
@ranocha
Copy link
Contributor Author

ranocha commented Mar 17, 2022

Bump

@timholy
Copy link
Member

timholy commented Mar 19, 2022

Can you add a test, please? I'm not quite sure what your comment about the test means. Can't you reduce it to a standalone MWE?

@ranocha
Copy link
Contributor Author

ranocha commented Mar 19, 2022

Right now, I only know minimal examples when running this in Jupyter notebooks or in Documenter.jl environments, not in "plain Julia" environments. Shall I add a Documenter setup just for testing this case?

Co-authored-by: Tim Holy <tim.holy@gmail.com>
@timholy
Copy link
Member

timholy commented Apr 18, 2022

You can do this:

ex = :(f_no_linenum(::Int) = 1)
deleteat!(ex.args[2].args, 1)    # delete the file & line number info
eval(ex)

and then run code_string on f_no_linenum.

Co-authored-by: Tim Holy <tim.holy@gmail.com>
@ranocha
Copy link
Contributor Author

ranocha commented Apr 19, 2022

Thanks for the suggestion! I added such a test.

test/runtests.jl Outdated
ex = :(f_no_linenum(::Int) = 1)
deleteat!(ex.args[2].args, 1) # delete the file & line number info
eval(ex)
@test_nowarn code_string(f_no_linenum, (Int,))
Copy link
Member

Choose a reason for hiding this comment

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

Should be @test code_string(f_no_linenum, (Int,)) == nothing or?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed in 65ff789

Co-authored-by: Kristoffer Carlsson <kristoffer.carlsson@juliacomputing.com>
@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #92 (65ff789) into master (b2e5fae) will decrease coverage by 31.04%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master      #92       +/-   ##
===========================================
- Coverage   88.06%   57.02%   -31.05%     
===========================================
  Files           3        3               
  Lines         243      242        -1     
===========================================
- Hits          214      138       -76     
- Misses         29      104       +75     
Impacted Files Coverage Δ
src/CodeTracking.jl 59.05% <100.00%> (-34.55%) ⬇️
src/utils.jl 49.48% <0.00%> (-31.52%) ⬇️
src/pkgfiles.jl 83.33% <0.00%> (-5.56%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2e5fae...65ff789. Read the comment docs.

@timholy timholy merged commit cbe863c into JuliaDebug:master Apr 19, 2022
@timholy
Copy link
Member

timholy commented Apr 19, 2022

Thanks!

@ranocha
Copy link
Contributor Author

ranocha commented Apr 19, 2022

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants