Repository: JEG2/highline (ruby)

Issue:
TypeError when calling say
When calling `HighLine#say` a `TypeError` is raised when printing the same number of lines as your `$terminal.page_at` value.

It can be reproduced with this gist:
https://gist.github.com/carbonin/4163f462345581956f4b

Traceback:

```
/home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:997:in `+': no implicit conversion of nil into Array (TypeError)
    from /home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:997:in `page_print'
    from /home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:724:in `format_statement'
    from /home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:621:in `say'
    from highline_bug.rb:9:in `<main>'
```

A fix would be to change the `lines.slice(-2,1)` to `[lines.last]` at `lib/highline.rb:997` which I think would get the desired behaviour.

The `$terminal.page_at` value can be increased as a temporary work around to prevent paging entirely, but that only works for me as my application typically has a fixed number of lines being printed.

TypeError when calling say
When calling `HighLine#say` a `TypeError` is raised when printing the same number of lines as your `$terminal.page_at` value.

It can be reproduced with this gist:
https://gist.github.com/carbonin/4163f462345581956f4b

Traceback:

```
/home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:997:in `+': no implicit conversion of nil into Array (TypeError)
    from /home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:997:in `page_print'
    from /home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:724:in `format_statement'
    from /home/ncarboni/.gem/ruby/2.2.2/gems/highline-1.7.7/lib/highline.rb:621:in `say'
    from highline_bug.rb:9:in `<main>'
```

A fix would be to change the `lines.slice(-2,1)` to `[lines.last]` at `lib/highline.rb:997` which I think would get the desired behaviour.

The `$terminal.page_at` value can be increased as a temporary work around to prevent paging entirely, but that only works for me as my application typically has a fixed number of lines being printed.



Discussion:
@carbonin  I was not able to reproduce the issue with the code on your gist.

Could rerun the gist exactly as it is and confirm the issue?

Can you share the output of the following?

```
$ ruby -r highline -ve 'p [HighLine::VERSION, HighLine::SystemExtensions::CHARACTER_MODE]'
```

My output is as follow:

```
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin14]
["1.7.7", "stty"]
```

Oh, I forgot to mention that at the pagination prompt you have to press 'q' to stop.  Then you should get the traceback.

```
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-linux]
["1.7.7", "stty"]
```

Is the output from
`$ ruby -r highline -ve 'p [HighLine::VERSION, HighLine::SystemExtensions::CHARACTER_MODE]'`

Great, now I get the error.

If the fix is so simple, don't you want to fire a PR yourself? 
So you enter to the "hall of fame" of HighLine contributors :smiley: 

But, if you prefer I can do it.

The branch is `1-7-stable`

@carbonin 

If the sentence has the exact same lines of `HighLIne#page_at` maybe the proper behaviour would be "not even paginate", do you agree? (So it was a **double** bug ;-) )

I'm crafting the tests here and will push a patch soon.

Can I do PR to your fork branch so we merge it on our master altogether?

@carbonin  I was not able to reproduce the issue with the code on your gist.

Could rerun the gist exactly as it is and confirm the issue?

Can you share the output of the following?

```
$ ruby -r highline -ve 'p [HighLine::VERSION, HighLine::SystemExtensions::CHARACTER_MODE]'
```

My output is as follow:

```
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin14]
["1.7.7", "stty"]
```

Oh, I forgot to mention that at the pagination prompt you have to press 'q' to stop.  Then you should get the traceback.

```
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-linux]
["1.7.7", "stty"]
```

Is the output from
`$ ruby -r highline -ve 'p [HighLine::VERSION, HighLine::SystemExtensions::CHARACTER_MODE]'`

Great, now I get the error.

If the fix is so simple, don't you want to fire a PR yourself? 
So you enter to the "hall of fame" of HighLine contributors :smiley: 

But, if you prefer I can do it.

The branch is `1-7-stable`

@carbonin 

If the sentence has the exact same lines of `HighLIne#page_at` maybe the proper behaviour would be "not even paginate", do you agree? (So it was a **double** bug ;-) )

I'm crafting the tests here and will push a patch soon.

Can I do PR to your fork branch so we merge it on our master altogether?


Patch:
diff --git a/lib/highline/paginator.rb b/lib/highline/paginator.rb
--- a/lib/highline/paginator.rb
+++ b/lib/highline/paginator.rb
@@ -19,12 +19,12 @@ def initialize(highline)
     def page_print(text)
       return text unless highline.page_at
 
-      lines = text.scan(/[^\n]*\n?/)
+      lines = text.lines.to_a
       while lines.size > highline.page_at
         highline.puts lines.slice!(0...highline.page_at).join
         highline.puts
         # Return last line if user wants to abort paging
-        return (["...\n"] + lines.slice(-2,1)).join unless continue_paging?
+        return "...\n#{lines.last}" unless continue_paging?
       end
       return lines.join
     end


Test patch:
diff --git a/test/test_paginator.rb b/test/test_paginator.rb
--- a/test/test_paginator.rb
+++ b/test/test_paginator.rb
@@ -27,4 +27,47 @@ def test_paging
                   (45..50).map { |n| "This is line #{n}.\n"}.join,
                   @output.string )
   end
+
+  def test_statement_lines_count_equal_to_page_at_shouldnt_paginate
+    @terminal.page_at = 6
+
+    @input << "\n"
+    @input.rewind
+
+    list = "a\nb\nc\nd\ne\nf\n"
+
+    @terminal.say(list)
+    assert_equal(list, @output.string)
+  end
+
+  def test_statement_with_one_line_bigger_than_page_at_should_paginate
+    @terminal.page_at = 6
+
+    @input << "\n"
+    @input.rewind
+
+    list = "a\nb\nc\nd\ne\nf\ng\n"
+
+    paginated =
+      "a\nb\nc\nd\ne\nf\n" \
+      "\n-- press enter/return to continue or q to stop -- \n\n" \
+      "g\n"
+
+    @terminal.say(list)
+    assert_equal(paginated, @output.string)
+  end
+
+  def test_quiting_paging_shouldnt_raise
+    # See https://github.com/JEG2/highline/issues/168
+
+    @terminal.page_at = 6
+
+    @input << "q"
+    @input.rewind
+
+    list = "a\nb\nc\nd\ne\nf\n"
+
+    # expect not to raise an error on next line
+    @terminal.say(list)
+  end
 end
\ No newline at end of file