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

Add passthrough of non-Markdown docs in document trimming #36091

Merged
merged 1 commit into from
Jun 26, 2020

Conversation

jmert
Copy link
Contributor

@jmert jmert commented May 31, 2020

The new extended docs feature added in #34226 defined how to trim [Julia's standard] Markdown docs. This breaks the docs retrieved in PyPlot since it just forwards through the original Python docs as plain-text (wrapped in Base.Docs.Text), and there's no appropriate trimdocs() method for that case. There is a fallback definition of the internal _trimdocs(), though, so it appears the restriction to specifically trimdocs(::Markdown.MD, ::Bool) may have been a mistake, and loosening the type on the first argument fixes the issue.

I've tested this fix on master, but it should be noted that the new v1.5 beta throws the same error when using PyPlot, so this PR should probably be backported (if accepted).

julia> using PyPlot                                                                                                                                                                                                
                                                                                                                                                                                                                   
help?> plot                                                                                                                                                                                                        
search: plot plot3D plotfile plot_date plot_trisurf plot_surface plot_wireframe triplot subplot boxplot

ERROR: MethodError: no method matching trimdocs(::Text{PyPlot.var"#1#2"{Tuple{PyPlot.LazyHelp}}}, ::Bool)
Closest candidates are:                             
  trimdocs(::Markdown.MD, ::Bool) at /home/justin/devel/ext/julia/usr/share/julia/stdlib/v1.6/REPL/src/docview.jl:111
Stacktrace:                                         
 [1] top-level scope at /home/justin/devel/ext/julia/usr/share/julia/stdlib/v1.6/REPL/src/docview.jl:366

julia> using REPL; Revise.track(REPL)

help?> plot                                         
search: plot plot3D plotfile plot_date plot_trisurf plot_surface plot_wireframe triplot subplot boxplot subplots matplotlib subplot_tool subplot2grid subplots_adjust get_plot_commands stackplot eventplot

Plot y versus x as lines and/or markers.

Call signatures::

    plot([x], y, [fmt], *, data=None, **kwargs)
    plot([x], y, [fmt], [x2], y2, [fmt2], ..., **kwargs)
[...]

@jmert jmert changed the title Add pass-through definitions of text/HTML in extended docs trimming Loosen type signature to allow passthrough of non-Markdown docs May 31, 2020
@ViralBShah ViralBShah added the docsystem The documentation building system label Jun 1, 2020
@jmert
Copy link
Contributor Author

jmert commented Jun 5, 2020

Maybe @timholy should review since his PR added the feature?

@jmert
Copy link
Contributor Author

jmert commented Jun 11, 2020

bump

stdlib/REPL/src/docview.jl Outdated Show resolved Hide resolved
stdlib/REPL/src/docview.jl Outdated Show resolved Hide resolved
@jmert jmert changed the title Loosen type signature to allow passthrough of non-Markdown docs Add passthrough of non-Markdown docs in document trimming Jun 12, 2020
Copy link
Member

@tkf tkf left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@jmert
Copy link
Contributor Author

jmert commented Jun 20, 2020

Just another friendly bump. This would be nice to get onto the v1.5 branch.

@tkf tkf added backport 1.5 kind:bugfix This change fixes an existing bug labels Jun 20, 2020
@tkf
Copy link
Member

tkf commented Jun 20, 2020

I think it's fair to say this is a bug fix and backport is worth considering. So I'm adding the backport labels.

@timholy @fredrikekre It'd be great if one of you can have a look at it.

@KristofferC KristofferC mentioned this pull request Jun 25, 2020
9 tasks
@fredrikekre fredrikekre merged commit 063525f into JuliaLang:master Jun 26, 2020
KristofferC pushed a commit that referenced this pull request Jun 26, 2020
mbauman added a commit to dlfivefifty/julia that referenced this pull request Jun 26, 2020
* origin/master: (232 commits)
  Add passthrough for non-Markdown docs (JuliaLang#36091)
  Fix pointer to no longer assume contiguity (JuliaLang#36405)
  Ensure string-hashing is defined before it gets used (JuliaLang#36411)
  Make compilecache atomic (JuliaLang#36416)
  add a test for JuliaLang#30739 (JuliaLang#36395)
  Fix broken links in docstring of `repeat` (JuliaLang#36376)
  fix and de-dup cached calls to `methods_by_ftype` in compiler (JuliaLang#36404)
  ml-matches: skip unnecessary work, when possible (JuliaLang#36413)
  gf: fix some issues with the move from using a tree to a hash lookup of leaf types (JuliaLang#36413)
  Add news and manual entry for sincospi (JuliaLang#36403)
  Check axes in Array(::AbstractArray) (fixes JuliaLang#36220) (JuliaLang#36397)
  add versions of `code_typed` and `which` that accept tuple types (JuliaLang#36389)
  Fix spelling of readdir. (JuliaLang#36409)
  add sincospi (JuliaLang#35816)
  fix showing methods with unicode gensymed variable names (JuliaLang#36396)
  Add doctest: eachslice (JuliaLang#36386)
  fix documentation typo ("Ingeger")
  Refactor `abstract_eval` to separate out statements and values (JuliaLang#36350)
  fix return type of `get!` on `IdDict` (JuliaLang#36383)
  Allow single option with REPL.TerminalMenus (JuliaLang#36369)
  ...
@jmert jmert deleted the trimdocs_text branch June 26, 2020 18:03
@KristofferC KristofferC mentioned this pull request Jul 8, 2020
13 tasks
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docsystem The documentation building system kind:bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants