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) Show now handles ; in a tuple #33627

Closed
wants to merge 5 commits into from

Conversation

lhnguyen-vn
Copy link
Contributor

Fix #32775

@c42f c42f added the display and printing Aesthetics and correctness of printed representations of objects. label Oct 22, 2019
Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

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

Cool, generally seems useful to have more round-trippable syntax 👍

It seems there's an out of bounds bug somewhere here which is causing CI failures. Might be because args can be length 0?

base/show.jl Outdated
@@ -1168,7 +1168,13 @@ function show_unquoted(io::IO, ex::Expr, indent::Int, prec::Int)
sep = ", "
end
head !== :row && print(io, op)
show_list(io, args, sep, indent)
if isa(args[1], Expr) && args[1].head === :parameters
Copy link
Member

Choose a reason for hiding this comment

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

You can use Meta.isexpr(args[1], :parameters) for this.

@lhnguyen-vn
Copy link
Contributor Author

Thanks for the response! I totally forgot about empty tuples, it has been updated now.

@c42f
Copy link
Member

c42f commented Oct 24, 2019

Oh, it appears that Meta.isexpr is not available during bootstrap which has caused CI to fail (even though it's used elsewhere in this file, those code paths must not hit during bootstrap).

The easiest fix is to go back to using the explicit version without Meta.isexpr... sorry!

test/show.jl Outdated
@@ -753,6 +753,9 @@ test_mt(show_f5, "show_f5(A::AbstractArray{T,N}, indices::Vararg{$Int,N})")
@test_repr "[a;]"
@test_repr "[a; b]"

# Issue #32775, printing ; in a tuple
@test sprint(show, :((a,;b))) == ":((a; b))"
Copy link
Member

@JeffBezanson JeffBezanson Oct 28, 2019

Choose a reason for hiding this comment

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

This printing is incorrect; (a,; b) and (a; b) are different (the comma makes it a tuple).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I'm missing something, but wouldn't Julia parse (a; b) as a block?

Copy link
Member

Choose a reason for hiding this comment

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

Right, that's the point—these syntaxes are not equivalent:

julia> Meta.@dump (a; b)
Expr
  head: Symbol block
  args: Array{Any}((3,))
    1: Symbol a
    2: LineNumberNode
      line: Int64 1
      file: Symbol REPL[53]
    3: Symbol b

julia> Meta.@dump (a,; b)
Expr
  head: Symbol tuple
  args: Array{Any}((2,))
    1: Expr
      head: Symbol parameters
      args: Array{Any}((1,))
        1: Symbol b
    2: Symbol a

@JeffBezanson
Copy link
Member

Taken care of by #34038

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
display and printing Aesthetics and correctness of printed representations of objects.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improve show to handle ; in a tuple
4 participants