JEG2 / highline

A higher level command-line oriented interface.
Other
1.29k stars 137 forks source link

TypeError when calling say #168

Closed carbonin closed 9 years ago

carbonin commented 9 years ago

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.

abinoam commented 9 years ago

@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"]
carbonin commented 9 years ago

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]'

abinoam commented 9 years ago

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.

abinoam commented 9 years ago

The branch is 1-7-stable

abinoam commented 9 years ago

@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?