Store hist --replay <i> calls in history

Fix issue #484 (hist --replay N isn't stored in history).

First, make some small amendments to existing code:

  * Make helper methods of "hist" command private;

  * What an irony! Amend the name of the duplicated test in
    `test_input.rb` ("should not contain duplicated lines" test).

Secondly, resolve the issue. There is one notable moment in current
implementation. Although `hist --replay` calls are being stored in
history, you cannot "replay" entries of this kind (you cannot replay
another call request to replay). Let me show an example:

  [1] pry(main)> hist --show 46894
  46894: hist --replay 46675..46677
  [2] pry(main)> hist --show 46675..46677
  46675: 1+1
  46676: a = 100
  46677: hist --tail
  [3] pry(main)> hist --replay 46894
  Error: Replay index 46894 points out to another replay call: `hist -r 46675..46677`
  [4] pry(main)>

There are two reasons for that.

Reason one or my incompetence
-----------------------------

First of all, I simply failed to implement such behaviour. With current
state of things (that are introduced in this commit), if you do not
raise `Pry::CommandError`, you cannot guarantee that only user's input
is getting stored in history. Here's an example when we get unwanted
entry in history:

  [1] pry(main)> hist --show 46894
  46894: hist --replay 46675..46677
  [2] pry(main)> hist --show 46675..46677
  46675: 1+1
  46676: a = 100
  46677: hist --tail 4
  [3] pry(main)> hist --replay 46894
  => 2
  => 100
  47021: hist --show 46894
  47022: hist --show 46675..46677
  47023: hist --replay 46894
  47024: hist --replay 46675..46677
  [8] pry(main)>

Note that a user typed only `hist --replay 46894`. But the last saved
entry in history is the entry to which user's input, actually, pointed
out (`hist --replay 46675..46677`). So if you press up-arrow key, you
will get not what you expected.

Reason two or "Whoa, whoa, boy! There is a real reason"
-------------------------------------------------------

But the main reason is that you can fall into a loop trap, when both
"hist --replay" calls point to each other. Example of a loop trap:

  [31] pry(main)> hist --tail 4
  47027: hist --tail
  47028: hist --replay 47028
  47029: hist --tail
  47030: hist --replay 47032
  [32] pry(main)> hist -r 47030
  # We've just fallen into a loop trap. Let's break out of it.
  ^C
  [416] pry(main)> hist --tail 5
  47409: hist --replay 47032
  47410: hist --replay 47030
  47411: hist --replay 47032
  47412: hist --replay 47030
  47413: hist --replay 47032
  [417] pry(main)>

Note the number of current line (417).

Finally, add some unit tests for this commit.

Signed-off-by: Kyrylo Silin <kyrylosilin@gmail.com>

lib/pry/commands/hist.rb

@@ -65,6 +65,8 @@ class Pry
       end
     end
 
+    private
+
     def process_display
       unless opts.present?(:'no-numbers')
         @history = @history.with_line_numbers
@@ -101,12 +103,52 @@ class Pry
 
     def process_replay
       @history = @history.between(opts[:r])
+      replay_sequence = @history.raw
+
+      check_for_juxtaposed_replay(replay_sequence)
 
       _pry_.input_stack.push _pry_.input
-      _pry_.input = StringIO.new(@history.raw)
+      _pry_.input = StringIO.new(replay_sequence)
       # eval_string << "#{@history.raw}\n"
       # run "show-input" unless _pry_.complete_expression?(eval_string)
     end
+
+    # If we met another an extra "hist" call to be replayed, check for the
+    # "--replay" option presence. If "hist" command is called with other options
+    # then proceed further. Example:
+    #   [1] pry(main)> hist --show 46894
+    #   46894: hist --replay 46675..46677
+    #   [2] pry(main)> hist --show 46675..46677
+    #   46675: 1+1
+    #   46676: a = 100
+    #   46677: hist --tail
+    #   [3] pry(main)> hist --replay 46894
+    #   Error: Replay index 46894 points out to another replay call: `hist -r 46675..46677`
+    #   [4] pry(main)>
+    #
+    # @raise [Pry::CommandError] If +replay_sequence+ contains another
+    #   "hist --replay" call
+    # @param [String] replay_sequence The sequence of commands to be replayed
+    #   (per saltum)
+    # @return [Boolean] `false` if +replay_sequence+ does not contain another
+    #   "hist --replay" call
+    def check_for_juxtaposed_replay(replay_sequence)
+      if replay_sequence =~ /\Ahist(?:ory)?\b/
+        # Create *fresh* instance of Slop for parsing of "hist" command.
+        _slop = self.slop
+        _slop.parse replay_sequence.split(" ")[1..-1]
+
+        if _slop.present?(:r)
+          replay_sequence = replay_sequence.split("\n").join("; ")
+          index = opts[:r]
+          index = index.min if index.min == index.max
+
+          raise CommandError, "Replay index #{ index } points out to another replay call: `#{ replay_sequence }`"
+        end
+      else
+        false
+      end
+    end
   end
 
   Pry::Commands.alias_command "history", "hist"

lib/pry/pry_instance.rb

@@ -409,7 +409,15 @@ class Pry
         eval_string << "#{indented_val.chomp}\n" unless val.empty?
       end
     ensure
-      Pry.history << indented_val unless input.is_a?(StringIO)
+      if input.is_a?(StringIO)
+        # Add to history only those values that were typed by a user. Ignore
+        # programmatically created ones.
+        unless input.string.include?(indented_val)
+          Pry.history << indented_val.chomp
+        end
+      else
+        Pry.history << indented_val
+      end
     end
   end
 

test/test_default_commands/test_input.rb

@@ -428,7 +428,7 @@ describe "Pry::DefaultCommands::Input" do
       @str_output.string.each_line.grep(/_ \+= 1/).count.should == 1
     end
 
-    it "should not contain duplicated lines" do
+    it "should not contain empty lines" do
       redirect_pry_io(InputTester.new(":place_holder", "2 + 2", "", "", "3 + 3", "hist", "exit-all"), @str_output) do
         pry
       end
@@ -438,5 +438,46 @@ describe "Pry::DefaultCommands::Input" do
 
       (a + 1).should == b
     end
+
+    it "should store a call with `--replay` flag" do
+      redirect_pry_io(InputTester.new(":banzai", "hist --replay 1",
+                                      "hist", "exit-all"), @str_output) do
+        Pry.start
+      end
+
+      @str_output.string.should =~ /hist --replay 1/
+    end
+
+    it "should not contain lines produced by `--replay` flag" do
+      redirect_pry_io(InputTester.new(":banzai", ":geronimo", ":huzzah",
+                                      "hist --replay 1..3", "hist",
+                                      "exit-all"), @str_output) do
+        Pry.start
+      end
+
+      @str_output.string.each_line.to_a.reject { |line| line.start_with?("=>") }.size.should == 4
+      @str_output.string.each_line.to_a.last.should =~ /hist --replay 1\.\.3/
+      @str_output.string.each_line.to_a[-2].should =~ /:huzzah/
+    end
+
+    it "should raise CommandError when index of `--replay` points out to another `hist --replay`" do
+      redirect_pry_io(InputTester.new(":banzai", "hist --replay 1",
+                                      "hist --replay 2", "exit-all"), @str_output) do
+        Pry.start
+      end
+
+      @str_output.string.should =~ /Replay index 2 points out to another replay call: `hist --replay 1`/
+    end
+
+    it "should disallow execution of `--replay <i>` when CommandError raised" do
+      redirect_pry_io(InputTester.new("a = 0", "a += 1", "hist --replay 2",
+                                      "hist --replay 3", "'a is ' + a.to_s",
+                                      "hist", "exit-all"), @str_output) do
+        Pry.start
+      end
+
+      @str_output.string.each_line.to_a.reject { |line| line !~ /\A\d/ }.size.should == 5
+      @str_output.string.should =~ /a is 2/
+    end
   end
 end