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

Fix string interpolation into Markdown.Table and Markdown.List #37130

Merged
merged 1 commit into from
Aug 20, 2020

Conversation

fredrikekre
Copy link
Member

Fix string interpolation into Markdown.Table and Markdown.List in md"..." strings, fixes #16194.

@@ -11,7 +11,7 @@ function interpinner(stream::IO, greedy = false)
startswith(stream, '$') || return
(eof(stream) || peek(stream, Char) in whitespace) && return
try
return _parse(stream::IOBuffer, greedy = greedy)
return _parse(stream, greedy = greedy)
Copy link
Member Author

Choose a reason for hiding this comment

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

This type-assert in try-catch threw sometimes since in some cases this function is passed a GenericIOBuffer instead of an IOBuffer....

Copy link
Member

Choose a reason for hiding this comment

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

Can you try this? (Requires MethodAnalysis 0.3):

julia> using Markdown, MethodAnalysis

julia> methodinstances(Markdown.interpinner)
2-element Vector{Core.MethodInstance}:
 MethodInstance for interpinner(::IOBuffer)
 MethodInstance for interpinner(::IOBuffer, ::Bool)

Just to make sure it doesn't get inferred with non-concrete IO type. (Maybe do it after running your tests.)

Copy link
Member Author

@fredrikekre fredrikekre Aug 20, 2020

Choose a reason for hiding this comment

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

julia> using Markdown, MethodAnalysis

julia> methodinstances(Markdown.interpinner)
2-element Vector{Core.MethodInstance}:
 MethodInstance for interpinner(::IOBuffer)
 MethodInstance for interpinner(::IOBuffer, ::Bool)

julia> begin
           x = "X"
           contains_X(md) = occursin(x, sprint(show, MIME("text/plain"), md))
           @assert contains_X(md"# $x") # H1
           @assert contains_X(md"## $x") # H2
           @assert contains_X(md"### $x") # H3
           @assert contains_X(md"x = $x") # Paragraph
           @assert contains_X(md"- $x") # List
           @assert contains_X(md"[$x](..)") # Link
           @assert contains_X(md"**$x**") # Bold
           @assert contains_X(md"*$x*") # Italic
           @assert contains_X( # Table
               md"""
               | name |
               |------|
               |  $x  |
               """)
       end

julia> methodinstances(Markdown.interpinner)
4-element Vector{Core.MethodInstance}:
 MethodInstance for interpinner(::IOBuffer)
 MethodInstance for interpinner(::Base.GenericIOBuffer{SubArray{UInt8,1,Vector{UInt8},Tuple{UnitRange{Int64}},true}})
 MethodInstance for interpinner(::IOBuffer, ::Bool)
 MethodInstance for interpinner(::Base.GenericIOBuffer{SubArray{UInt8,1,Vector{UInt8},Tuple{UnitRange{Int64}},true}}, ::Bool)

@thofma
Copy link
Contributor

thofma commented Aug 20, 2020

Does this by accident also fix #36946?

@fredrikekre
Copy link
Member Author

No.

@fredrikekre fredrikekre merged commit d446c27 into master Aug 20, 2020
@fredrikekre fredrikekre deleted the fe/md-interp branch August 20, 2020 21:16
KristofferC pushed a commit that referenced this pull request Aug 26, 2020
@KristofferC KristofferC mentioned this pull request Aug 26, 2020
29 tasks
KristofferC pushed a commit that referenced this pull request Aug 26, 2020
KristofferC pushed a commit that referenced this pull request Aug 26, 2020
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 29, 2020
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.

Markdown strings and interpolation
4 participants