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

Append newline to the end of dump methods #17202

Merged
merged 2 commits into from
Jul 7, 2016

Conversation

omus
Copy link
Member

@omus omus commented Jun 29, 2016

The doctest system doesn't deal very well with examples that don't have a trailing newline. Originally mentioned in #17106 and in the julia-dev forums

This is my first time making modifications to the dump methods. Let me know if there's something I didn't consider.

@@ -1158,6 +1168,7 @@ function dumptype(io::IO, x::ANY, n::Int, indent)
end
end
end
isempty(indent) && println(io)
end
Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe all the dump methods should explicitly return nothing?

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Yes; otherwise they return false which doesn't really make sense.

@omus omus mentioned this pull request Jun 29, 2016
@kshyatt
Copy link
Contributor

kshyatt commented Jul 3, 2016

LGTM. Anyone else have thoughts?

@omus
Copy link
Member Author

omus commented Jul 7, 2016

Would be nice to get this merged.

@@ -500,3 +500,9 @@ let s = IOBuffer(Array{UInt8}(0), true, true)
Base.showarray(s, [1,2,3], false, header = false)
@test String(resize!(s.data, s.size)) == " 1\n 2\n 3"
end

# The `dump` function should alway have a trailing newline
Copy link
Contributor

Choose a reason for hiding this comment

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

always

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. I'll take care of that in another PR

@JeffBezanson JeffBezanson merged commit a9c2e27 into JuliaLang:master Jul 7, 2016
@omus omus deleted the dump-trailing-newline branch July 8, 2016 03:05
omus added a commit to omus/julia that referenced this pull request Jul 8, 2016
@omus omus mentioned this pull request Jul 8, 2016
tkelman added a commit that referenced this pull request Jul 8, 2016
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
* Append newline to the end of dump methods

* Made dump methods return nothing
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
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.

4 participants