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 #17956 and add tests #18012

Merged
merged 5 commits into from
Aug 14, 2016
Merged

Fix #17956 and add tests #18012

merged 5 commits into from
Aug 14, 2016

Conversation

dhoegh
Copy link
Contributor

@dhoegh dhoegh commented Aug 13, 2016

This fixes #17956 and add tests for the bug. The bug was introduced in 103db50 cc: @StefanKarpinski

PR commits changes:

  1. Move env.jl test to it own file in test
  2. Fix Printing of ENV not displaying properly #17956
  3. Deprecate delete!(ENV, k ,def) as it is inherently type unstable and no other associative type implement the method.
  4. Move docs for withenv inline

The deprecation of delete!(ENV, k ,def) should not be backported, but the rest of the commits should.

@tkelman tkelman added backport pending 0.5 bugfix This change fixes an existing bug labels Aug 13, 2016
io = IOBuffer()
show(io, ENV)
s = takebuf_string(io)
@test contains(s, k1)
Copy link
Contributor

Choose a reason for hiding this comment

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

should we be even more specific and check for "$k1 => $k1" (maybe with some whitespace padding as needed)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated it to:

    @test contains(s, "$k1=$k1")
    @test contains(s, "$k2=$k2")

@tkelman tkelman merged commit 0b7c8c2 into JuliaLang:master Aug 14, 2016
@tkelman
Copy link
Contributor

tkelman commented Aug 14, 2016

Thank you! Good catch. Off-by-one errors are annoying.

tkelman pushed a commit that referenced this pull request Aug 20, 2016
tkelman pushed a commit that referenced this pull request Aug 20, 2016
tkelman pushed a commit that referenced this pull request Aug 20, 2016
(cherry picked from commit bb7272e)
ref #18012
tkelman pushed a commit that referenced this pull request Aug 20, 2016
@Sacha0 Sacha0 added deprecation This change introduces or involves a deprecation needs news A NEWS entry is required for this change labels May 14, 2017
@tkelman tkelman removed the needs news A NEWS entry is required for this change label May 16, 2017
tkelman pushed a commit that referenced this pull request May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug deprecation This change introduces or involves a deprecation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Printing of ENV not displaying properly
3 participants