[ruby-core:95008] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[ruby-core:95008] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[burdettelamar]

Issue #16173 has been reported by burdettelamar@yahoo.com (Burdette Lamar).

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (961 Bytes)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95009] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[XrXr]

Issue #16173 has been updated by alanwu (Alan Wu).


This gives the impression that the return value is always nil, which isn't true when the environment variable exists.
Maybe it's worth clarifying that.

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81637

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (961 Bytes)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95012] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[nobu]

Issue #16173 has been updated by nobu (Nobuyoshi Nakada).


Comparing with `Hash#delete`, it looks that the document is correct and the code is wrong.

```diff
diff --git i/hash.c w/hash.c
index 8b84a14484..7880178dc8 100644
--- i/hash.c
+++ w/hash.c
@@ -4779,7 +4779,7 @@ env_delete_m(VALUE obj, VALUE name)
     VALUE val;
 
     val = env_delete(name);
-    if (NIL_P(val) && rb_block_given_p()) rb_yield(name);
+    if (NIL_P(val) && rb_block_given_p()) val = rb_yield(name);
     return val;
 }
 
diff --git i/test/ruby/test_env.rb w/test/ruby/test_env.rb
index b01c3b12ee..1a7656ea7d 100644
--- i/test/ruby/test_env.rb
+++ w/test/ruby/test_env.rb
@@ -107,6 +107,7 @@
     assert_invalid_env {|v| ENV.delete(v)}
     assert_nil(ENV.delete("TEST"))
     assert_nothing_raised { ENV.delete(PATH_ENV) }
+    assert_equal("NO TEST", ENV.delete("TEST") {|name| "NO "+name})
   end
 
   def test_getenv
```


----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81639

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (961 Bytes)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95017] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[burdettelamar]

Issue #16173 has been updated by burdettelamar@yahoo.com (Burdette Lamar).


Thanks, @nobu, but I'm not going to propose a change to the functionality.

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81644

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (961 Bytes)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95019] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[burdettelamar]

Issue #16173 has been updated by burdettelamar@yahoo.com (Burdette Lamar).

File diff.txt added

Thanks, @alanwu.  I'm refreshing diff.txt with more fulsome documentation, along with enhanced testing.

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81646

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (961 Bytes)
diff.txt (2.22 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95038] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[eregontp]

Issue #16173 has been updated by Eregon (Benoit Daloze).


IMHO it's better to fix behavior to be consistent with Hash#delete.
And the compatibility risk seems non existent here.

So I think we should change behavior, and not change documentation which would be inconsistent with Hash#delete.
+1 for @nobu's patch.

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81669

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (2.22 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95063] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[burdettelamar]

Issue #16173 has been updated by burdettelamar@yahoo.com (Burdette Lamar).


I, being a member only of The Outer Party, can't merge anything here.

I'm agnostic on whether the change should be to the documentation or to the code.  My part was just to note the discrepancy.

Can someone in The Inner Party take action?

Thanks,
Burdette

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81696

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (2.22 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95065] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[merch-redmine]

Issue #16173 has been updated by jeremyevans0 (Jeremy Evans).


I also agree that nobu's patch should be merged.

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81698

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (2.22 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95166] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[nobu]

Issue #16173 has been updated by nobu (Nobuyoshi Nakada).


burdettelamar@yahoo.com (Burdette Lamar) wrote:
> Thanks, @alanwu.  I'm refreshing diff.txt with more fulsome documentation, along with enhanced testing.

Thank you for the patch.

```diff
+ * Deletes the environment variable for +name+ if it exists (ignoring the block, if given); returns +nil+:
+ *   ENV.delete('LINES') # => '300'
+ *   ENV.delete('COLUMNS') { |name| fail 'boo' } # => '120'
```

It should return the old value, not `nil`.

```diff
+ * Calls the block and returns +nil+ if the environment variable does not exist and block given:
+ *   ENV.delete('NOSUCH') { |name| } # => nil
```

Non-empty block and non-nil result feels better to me.

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81803

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (2.22 KB)


-- 
https://bugs.ruby-lang.org/

[ruby-core:95170] [Ruby master Bug#16173] ENV.delete returns nil when name does not exist and block given

[nobu]

Issue #16173 has been updated by nobu (Nobuyoshi Nakada).


https://github.com/nobu/ruby/pull/new/bug/16173-ENV.delete-block

----------------------------------------
Bug #16173: ENV.delete returns nil when name does not exist and block given
https://bugs.ruby-lang.org/issues/16173#change-81806

* Author: burdettelamar@yahoo.com (Burdette Lamar)
* Status: Open
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: ruby 2.6.4p104 (2019-08-28 revision 67798) [x64-mingw32]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN
----------------------------------------
Attached diff.txt:

* ENV.delete for nonexistent name and block given:
  * Test enhanced to verify return value is nil.
  * Documentation corrected to say that return value is nil, not value.


---Files--------------------------------
diff.txt (2.22 KB)


-- 
https://bugs.ruby-lang.org/