update from reviews

README.md

@@ -38,8 +38,8 @@ Features
   * Piping commands together
   * 100% secure (shell expansion is impossible by design)
   * Raises exceptions on errors (no more manual status code checks)
-  * allow specification of valid exit codes
-  * allow temporary overwrite of env variables
+    but allows to specify which non-zero codes are still a success run
+  * Allows overriding environment variables
   * Optional logging for easy debugging
 
 Non-features
@@ -152,9 +152,12 @@ status, input and both outputs to it:
 Cheetah.run("ls -l", logger: logger)
 ```
 
-### Overwritting env
+### Overwriting env
 
-If command need to have own specified ENV set, then passing :env option does it.
+If the command needs adapted environment variables, use the :env option.
+Passed hash is used to update existing env (for details see ENV.update).
+Nil value mean unset varible. Environment is return back to original state after
+run of a command.
 
 ```ruby
   Cheetah.run("env", env: { "LC_ALL" => "C" })

lib/cheetah.rb

@@ -215,7 +215,8 @@ def log_stream_line(stream, line)
     stdin:  "",
     stdout: nil,
     stderr: nil,
-    logger: nil
+    logger: nil,
+    env:    {}
   }
 
   READ  = 0 # @private
@@ -304,9 +305,9 @@ class << self
     #   @option options [Fixnum, .include?, nil] :allowed_exitstatus (nil)
     #     Allows to specify allowed exit codes that do not cause exception. It
     #     adds as last element of result exitstatus.
-    #   @option options [Hash] :env (nil)
-    #     Allows to overwrite env for running command. if key have nil value it
-    #     is deleted from env.
+    #   @option options [Hash] :env ({})
+    #     Allows to update ENV for the time of running the command. if key maps to nil value it
+    #     is deleted from ENV.
     #
     #   @example
     #     Cheetah.run("tar", "xzf", "foo.tar.gz")
@@ -378,7 +379,6 @@ class << self
 
     def run(*args)
       options = args.last.is_a?(Hash) ? args.pop : {}
-      return changed_env(options, *args) if options[:env]
       options = BUILTIN_DEFAULT_OPTIONS.merge(@default_options).merge(options)
 
       options[:stdin] ||= "" # allow passing nil stdin see issue gh#11
@@ -393,28 +393,27 @@ def run(*args)
 
       recorder.record_commands(commands)
 
-      pid, pipes = fork_commands(commands)
-      select_loop(streams, pipes, recorder)
-      _pid, status = Process.wait2(pid)
+      with_env(options[:env]) do
+        pid, pipes = fork_commands(commands)
+        select_loop(streams, pipes, recorder)
+        _pid, status = Process.wait2(pid)
 
-      begin
-        check_errors(commands, status, streams, streamed, options)
-      ensure
-        recorder.record_status(status)
-      end
+        begin
+          check_errors(commands, status, streams, streamed, options)
+        ensure
+          recorder.record_status(status)
+        end
 
-      build_result(streams, status, options)
+        build_result(streams, status, options)
+      end
     end
 
     private
 
-    def changed_env(options, *args)
+    def with_env(env, &block)
       old_env = ENV.to_hash
-      options = options.dup # do not modify original options
-      env = options.delete(:env)
       ENV.update(env)
-      args.push(options)
-      run(*args)
+      yield
     ensure
       ENV.replace(old_env)
     end

spec/cheetah_spec.rb

@@ -761,7 +761,7 @@ def create_command(source, name: "command")
       describe "changing environment" do
         let(:command) do
           create_command(<<-EOT)
-            echo -n $CHEETAH_TEST
+            echo -n ${CHEETAH_TEST?"NOT SET"}
           EOT
         end
 
@@ -776,6 +776,13 @@ def create_command(source, name: "command")
           Cheetah.run(command, stdout: :capture, env: { "CHEETAH_TEST" => "fail" })
           expect(ENV["CHEETAH_TEST"]).to eq "OK"
         end
+
+        it "key with nil unsets value" do
+          ENV["CHEETAH_TEST"] = "OK"
+          err, exit_code = Cheetah.run(command, stderr: :capture, env: { "CHEETAH_TEST" => nil }, allowed_exitstatus: 1)
+          expect(exit_code).to eq 1
+          expect(err).to match(/NOT SET/)
+        end
       end
     end
   end