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

RFC add markdown Table #9878

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions base/markdown/GitHub/GitHub.jl
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
include("table.jl")

@breaking true ->
function fencedcode(stream::IO, block::MD, config::Config)
startswith(stream, "```", padding = true) || return false
Expand Down Expand Up @@ -33,9 +35,8 @@ function github_paragraph(stream::IO, md::MD, config::Config)
return true
end

# TODO: tables

@flavor github [list, indentcode, blockquote, fencedcode, hashheader, github_paragraph,
@flavor github [list, indentcode, blockquote, fencedcode, hashheader,
github_table, github_paragraph,
linebreak, escapes, en_dash, inline_code, asterisk_bold,
asterisk_italic, image, link]

linebreak, escapes, en_dash, inline_code, asterisk_bold, asterisk_italic,
image, link]
170 changes: 170 additions & 0 deletions base/markdown/GitHub/table.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
type Table
rows::Vector{Vector{Any}}
align
end

function github_table(stream::IO, md::MD, config::Config)
withstream(stream) do
rows = Any[]
n = 0
align = :r # default is to align right
while !eof(stream)
n += 1
pos = position(stream)
skipwhitespace(stream)
line = readline(stream) |> chomp

if n == 1
pipe_border = line[1] == '|'
if !('|' in line)
return false
end
end

row = map(strip, split(line, "|"))
if pipe_border
if row[1] == row[end] == ""
row = row[2:end-1]
else
return false
end
end

if n == 2 && all(['-' in r && issubset(Set(r), Set(" -:"))
for r in row])
# handle possible --- line
align = Symbol[]
for r in row
if r[1] == ':'
if r[end] == ':'
push!(align, :c)
else
push!(align, :l)
end
else
if r[end] == ':'
push!(align, :r)
else
# default is align right
push!(align, :r)
end
end
end

elseif n == 1 || length(rows[1]) == length(row)
push!(rows, map(x -> parseinline(x, config), row))
elseif length(row) > 1
seek(stream, pos)
Copy link
Member

Choose a reason for hiding this comment

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

There's a strong practice in Markdown.jl of not handling the stream position explicitly. I guess it's not essential, but it would still be nice to see row parsing factored out to address that

Copy link
Member Author

Choose a reason for hiding this comment

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

It may be easier to refactor some of this after merging

break
else
return false
end
end
if length(rows) < 2
return false
end
push!(md, Table(rows, align))
return true
end
end

function html(io::IO, md::Table)
withtag(io, :table) do
for (i, row) in enumerate(md.rows)
withtag(io, :tr) do
for c in md.rows[i]
t = (i == 1) ? :th : :td
withtag(io, t) do
htmlinline(io, c)
end
end
end
end
end
end

function plain(io::IO, md::Table)
plain_cells = map(row -> map(plaininline, row), md.rows)
plain_lengths = map(row -> int(map(length, row)), plain_cells)
col_widths = reduce(max, plain_lengths)

for i in 1:length(md.rows)
for (j, text) in enumerate(plain_cells[i])
(j != 1) && print(io, " | ")
print(io, " " ^ (col_widths[j] - plain_lengths[i][j]))
print(io, text)
end
println(io)

if i == 1
for (j, w) in enumerate(col_widths)
if j != 1
print(io, " | ")
end
a = typeof(md.align) == Symbol ? md.align : md.align[j]
print(io, _dash(w, a))
end
println(io)
end
end
end

function _dash(width, align)
if align == :l
return ":" * "-" ^ max(3, width - 1)
elseif align == :r
return "-" ^ max(3, width - 1) * ":"
elseif align == :c
return ":" * "-" ^ max(3, width - 2) * ":"
else
throw(ArgumentError("Unrecognized alignment $align"))
end
end


function writemime(io::IO, ::MIME"text/latex", md::Table)
wrapblock(io, "tabular") do
if typeof(md.align) == Symbol
align = string(md.align) ^ length(md.rows[1])
else
align = md.align
end
println(io, "{$(join(align, " | "))}")
for (i, row) in enumerate(md.rows)
for (j, cell) in enumerate(row)
if j != 1
print(io, " & ")
end
latex_inline(io, cell)
end
println(io, " \\\\")
if i == 1
println("\\hline")
end
end
end
end

function term(io::IO, md::Table, columns)
plain_lengths = map(row -> int(map(x -> ansi_length(terminline(x)), row)),
md.rows)
col_widths = reduce(max, plain_lengths)

col_widths = max(col_widths, 3)
for (i, row) in enumerate(md.rows)
for (j, h) in enumerate(row)
(j != 1) && print(io, " ")
a = typeof(md.align) == Symbol ? md.align : md.align[j]
print_align(io, h, plain_lengths[i][j], col_widths[j], a)
end
println(io)

if i == 1
for (j, w) in enumerate(col_widths)
(j != 1) && print(io, " ")
print(io, "-" ^ w)
end
println(io)
end
end
end
2 changes: 1 addition & 1 deletion base/markdown/Julia/Julia.jl
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ We start by borrowing GitHub's `fencedcode` extension – more to follow.
include("interp.jl")

@flavor julia [blocktex, blockinterp, hashheader, list, indentcode, fencedcode,
blockquote, paragraph,
blockquote, github_table, paragraph,

linebreak, escapes, latex, interp, en_dash, inline_code, asterisk_bold,
asterisk_italic, image, link]
5 changes: 3 additions & 2 deletions base/markdown/parse/parse.jl
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,9 @@ function parseinline(stream::IO, config::Config)
return content
end

parseinline(s::String, c::Config) =
parseinline(IOBuffer(s), c)
parseinline(s::String, c::Config) = parseinline(IOBuffer(s), c)
# TODO remove once GH #9888 is fixed
parseinline{T}(s::SubString{T}, c::Config) = parseinline(convert(T, s), c)

parseinline(s) = parseinline(s, _config_)

Expand Down
1 change: 1 addition & 0 deletions base/markdown/render/latex.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ function writemime(io::IO, ::MIME"text/latex", md::List)
end
end


# Inline elements

function writemime(io::IO, ::MIME"text/latex", md::Plain)
Expand Down
20 changes: 10 additions & 10 deletions base/markdown/render/plain.jl
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,19 @@ function plaininline(io::IO, md...)
end

plaininline(io::IO, md::Vector) = !isempty(md) && plaininline(io, md...)

plaininline(io::IO, md::Image) = print(io, "![$(md.alt)]($(md.url))")

plaininline(io::IO, s::String) = print(io, s)

plaininline(io::IO, md::Bold) = plaininline(io, "**", md.text, "**")

plaininline(io::IO, md::Italic) = plaininline(io, "*", md.text, "*")

plaininline(io::IO, md::Image) = print(io, "![", plaininline(md.alt), "](",
md.url, ")")
plaininline(io::IO, md::Link) = print(io, "[", plaininline(md.text), "](",
md.url, ")")
plaininline(io::IO, md::Bold) = print(io, "**", plaininline(md.text), "**")
plaininline(io::IO, md::Italic) = print(io, "*", plaininline(md.text), "*")
Copy link
Member

Choose a reason for hiding this comment

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

Good to see the extra methods here but there are a lot of unnecessary stylistic changes – would be great to see the diff being as simple as possible

plaininline(io::IO, md::Code) = print(io, "`", md.code, "`")

plaininline(io::IO, s::String) = print(io, s)
plaininline(io::IO, x) = writemime(io, MIME"text/plain"(), x)

plaininline(s::String) = s
Copy link
Member

Choose a reason for hiding this comment

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

This method should be unnecessary, it's covered by the one below

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 just saves copying a string.

Copy link
Member

Choose a reason for hiding this comment

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

Strings are stored in generated markdown trees as Any["foo"] though, so this method will almost never be called – kind of a premature optimisation. Not a big deal either way though I guess.

plaininline(x) = sprint(plaininline, x)

# writemime

Base.writemime(io::IO, ::MIME"text/plain", md::MD) = plain(io, md)
16 changes: 16 additions & 0 deletions base/markdown/render/terminal/formatting.jl
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,22 @@ function print_centred(io::IO, s...; columns = 80, width = columns)
end
end

function print_align(io::IO, text, text_width, width, align=:r)
if align == :c # center
lpad = div(width - text_width, 2)
elseif align == :r # right
lpad = width - text_width
elseif align == :l # left
lpad = 0
else
throw(ArgumentError("Alignment $align not recognised"))
end
print(io, " " ^ lpad)
terminline(io, text)
print(io, " " ^ (width - lpad - text_width))
end


function centred(s, columns)
pad = div(columns - ansi_length(s), 2)
" "^pad * s
Expand Down
16 changes: 15 additions & 1 deletion test/markdown.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using Base.Markdown
import Base.Markdown: MD, Paragraph, Italic, Bold, plain, term, html
import Base.Markdown: MD, Paragraph, Italic, Bold, plain, term, html, Table, Code
import Base: writemime

# Basics
Expand Down Expand Up @@ -39,3 +39,17 @@ writemime(io::IO, m::MIME"text/plain", r::Reference) =
print(io, "$(r.ref) (see Julia docs)")

@test md"Behaves like $(ref(fft))" == md"Behaves like fft (see Julia docs)"

# GH tables
@test md"""a|b
1|2""" == MD(Table(Any[Any[Any["a"],Any["b"]];Any[Any["1"],Any["2"]]], :r))

table = md"""| a | b | c |
| :- | -: | - |
| d`gh`hg | hgh**jhj**ge | f |"""
@test table == MD(Table(Any[Any[Any["a"],Any["b"],Any["c"]],
Any[Any["d",Markdown.Code("","gh"),"hg"],
Any["hgh",Markdown.Bold(Any["jhj"]),"ge"],
Any["f"]]],
[:l, :r, :r]))
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to make the tests as human-readable as possible, e.g. you can write this as

MD(Table(Any[["a","b","c"],
             Any[["d",Code("gh"),"hg"],
                 ["hgh",Bold("jhj"),"ge"],
                 "f"]],
         [:l, :r, :r]))

(This actually doesn't work at the moment, which I think is due to a problem with print_align)