Skip to content

SystemPager does not work if less is not available on system #712

Closed
voxik opened this Issue Sep 15, 2012 · 34 comments

3 participants

@voxik
voxik commented Sep 15, 2012

SystemPager fails if less is not available on the system. This might be the case for Windows without MSys installed as well as Fedora's mock environment for example. It would be nice to have some fallback for these cases. Thank you.

@ghost
ghost commented Sep 15, 2012

SystemPager is an internal class. The public interface is Pager.page(content, :type), where :type is :simple, :system, or nil for the default choice (which is to use the system pager if we're not on JRuby). We'd need to be a little more smart about choosing what pager to use if we were to tackle this one. FWIW, for now, you can page through Pry::Pager.page(content, :simple).

edit: Typo's.

@ghost
ghost commented Sep 15, 2012

Perhaps making the pager be a configurable Proc that ran the pager would be useful in this case. you could simply re-configure pry to your case instead of us trying to cover all of them. /cc @banister @ConradIrwin @kyrylo

@voxik
voxik commented Sep 15, 2012

Actually, I was trying to use ri in pry-0.9.10 on windows, and I was hit by:

[1] pry(main)> ri save
Errno::ENOENT: No such file or directory - less -R -S -F -X
from C:/Users/vita/.pik/rubies/Ruby-193-tcs-20111204/lib/ruby/gems/1.9.1/gems/pry-0.9.10-x86-mingw32/lib/pry/helpers/base_helpers.rb:217:in `popen'
[2] pry(main)>

That was the initial reason to report this issue. I did not tested the master though ;) but from the code, I assume it suffers the same issue.

@ghost
ghost commented Sep 15, 2012

@voxik How does Pry.config.pager_proc = Proc.new { |buf| Pager.page(buf, :simple) } sound as a solution to your problem?

@ghost
ghost commented Sep 15, 2012

Your proc would only be run if Pry.config.pager was true-ish, by the way. I think that's reasonable though, since by default it is true.

@voxik
voxik commented Sep 15, 2012

@robgleeson Hm, where to place it? Not sure that every windows user wants to configure Pry.

@ghost ghost was assigned Sep 15, 2012
@ghost
ghost commented Sep 15, 2012

@voxik you'd need to do that in your .pryrc file if you wanted to automate the solution but you could do it once Pry has started as well. We could handle windows as the default case (i.e: detect windows and not use the system pager) but I think Pry.config.pager_proc as an idea is a good one. Thoughts?

Edit: typo's.

@ghost
ghost commented Sep 15, 2012

Okay, by default, Windows users won't have to deal with the system pager. We definitely want to make this configurable though, we can't cover every case but if we leave it up to the user to solve then it isn't a problem
anymore.

@voxik
voxik commented Sep 15, 2012

Why not detect if less is available? There might be windows users who have installed less [1] as well as Linux user who doesn't have it. Isn't there any possibility to automatically add the configuration line into the .pryrc once you detect that less is not available?

[1] http://gnuwin32.sourceforge.net/packages/less.htm

@ghost
ghost commented Sep 15, 2012

For those cases, we can simply let the user choose. We can't try to cover every edge-case, can we? Reasonable defaults with flexibility to change those is a good idea, IMHO.

@ghost
ghost commented Sep 15, 2012

we could try which less and check $?.exitstatus but then we're assuming which exists. next approach is scanning the filesystem. that's slow. i think user configurable is the way to go.

EDIT: horrible typo, i meant which, not less :-)

@voxik
voxik commented Sep 15, 2012

Well, with which less you would face the same issues, since which is not available on every platform ;) but you could check the $?.exitstatus of less and if it fails, then fallback to the :simple type.

@kyrylo
pry member
kyrylo commented Sep 15, 2012

@voxik, as a temporary solution you may want to use Pry.pager = false in your .pryrc.

@ghost
ghost commented Sep 15, 2012

@voxik I agree. Lets go with your solution.

@kyrylo
pry member
kyrylo commented Sep 15, 2012

Also, there is a big confusion in this issue. @robgleeson merely confused versions of Pry. He described the situation for HEAD. But @voxik is using v0.9.10. The fact of the matter is that code of pager was refactored after v0.9.10 release. :)

@voxik
voxik commented Sep 15, 2012

@kyrylo Not really ... I noticed the issue in 0.9.10 and then examined the code and reported the issue against master, although I did not tested master.

@ghost
ghost commented Sep 15, 2012

@voxik can you test master now?

@ghost Unknown added a commit that referenced this issue Sep 15, 2012
Robert Gleeson Don't try to use the 'less' pager on Windows.
Ref #712.
6a67ea4
@ghost Unknown added a commit that closed this issue Sep 15, 2012
Robert Gleeson Don't try to run the system pager if 'less' is not available.
Closes #712.
c8dc5d4
@ghost ghost closed this in c8dc5d4 Sep 15, 2012
@ghost Unknown added a commit that referenced this issue Sep 15, 2012
Robert Gleeson Platform agnostic.
Ref #712.
7d7ec9e
@ghost
ghost commented Sep 15, 2012

Sorry, I had 'origin' set as my remote. I pushed to my repository and not Prys :-) Whenever you get a chance, please try pry/master and let us know if your issues are solved. Thanks!

@voxik
voxik commented Sep 15, 2012

Hmm, with 7d7ec9e, I built and installed the gem and now I got:

[1] pry(main)> ri save
NoMethodError: undefined method `lesspipe' for Pry::Helpers::BaseHelpers:Module
from C:/Users/vita/.pik/rubies/Ruby-193-tcs-20111204/lib/ruby/gems/1.9.1/gems/pry-0.9.10/lib/pry/commands/ri.rb:24:in `page'
@ghost
ghost commented Sep 15, 2012

can you please try:

$ git clone git://github.com/pry/pry.git
$ cd pry
$ bundle install
$ rake pry
@ghost
ghost commented Sep 15, 2012

ah nevermind, it is a bug. i'll fix it in a second. other commands should work fine.

@kyrylo
pry member
kyrylo commented Sep 15, 2012

@voxik, by the way, in future we will drop ri command. Use show-doc (aka ?) and show-source (aka $) instead :)

@ghost
ghost commented Sep 15, 2012

@voxik okay, this bug is very specific to the 'ri' command. could you try another command, e.g: $ Pry, and let us know if it works?

@voxik
voxik commented Sep 15, 2012

@robgleeson It works it seems

$ Pry

From: C:/Users/vita/.pik/rubies/Ruby-193-tcs-20111204/lib/ruby/gems/1.9.1/gems/pry-0.9.10/lib/pry/pry_instance.rb @ line 24:
Number of lines: 731

class Pry

  attr_accessor :input
  attr_accessor :output
  attr_accessor :commands
  attr_accessor :print
  attr_accessor :exception_handler
  attr_accessor :input_stack
  attr_accessor :quiet
  alias :quiet? :quiet

  attr_accessor :custom_completions

  attr_accessor :binding_stack

  attr_accessor :last_result
  attr_accessor :last_file
  attr_accessor :last_dir

  attr_reader :last_exception

  attr_reader :input_array
  attr_reader :output_array

<page break> --- Press enter to continue ( q<enter> to break ) --- <page break>
q

@kyrylo just for curiosity, are show-doc and ri really interchangeable? They are not IMO.

@ghost
ghost commented Sep 15, 2012

yup, that works, i know the output as being from the :simple pager. I can confirm it is working on my Mac OSX computer as well, so I think this issue has been resolved. Thanks for reporting! :-)

@voxik
voxik commented Sep 15, 2012

But what about the ri? Will it be functional once the 'lesspipe' is fixed?

@kyrylo
pry member
kyrylo commented Sep 15, 2012

@kyrylo just for curiosity, are show-doc and ri really interchangeable? They are not IMO.

@voxik, why not?

@voxik
voxik commented Sep 15, 2012

@kyrylo Since I cannot test it and since ? Pry worked just fine in 0.9.10 so I just want make sure.

@kyrylo
pry member
kyrylo commented Sep 15, 2012

@voxik, well, you can test that, since @robgleeson pushed the fix.

#712 (comment)

$ git clone git://github.com/pry/pry.git
$ cd pry
$ bundle install
$ rake pry

@voxik voxik added a commit to voxik/pry that referenced this issue Sep 16, 2012
@voxik voxik Fix the less check (#712).
The exit status doesn't matter, it is always OK, but an exception is fired only when less is not available on the system.
41dbc0d
@voxik
voxik commented Sep 17, 2012

Thank you!

@banister
pry member

@voxik beware that ri is likely to be removed in the next release of Pry, though :) If you have arguments for ri staying, then make your case here :)

@voxik
voxik commented Sep 17, 2012

@banister Well, using ri Pry::CommandSet#sum gives me documentation where ? Pry::CommandSet#sum gives me error Error: undefined methodPry::CommandSet#sum'`. Not sure what should be replacement and why it should be removed.

@banister
pry member

@voxik ? only provides 'live' documentation -- that is documentation on methods that can be invoked in the repl. It appears that Pry::CommandSet#sum is a monkeypatch provided by rails, and that rails is not currently loaded so the method is unavailable. In my opinion, this is an unsatisfactory and potentially confusing feature of ri (that you can view docs/source for unavailable methods) and I find the ? behavior more intuitive.

The reason i do not want to keep both commands is that they really perform similar functions and new users become confused as to which one to use...it is my opinion that ? is a superior command due to its 'live' nature, but more so its ability to retrieve documentation on methods/classes that do not have docs generated. Relying on ? instead means users can forego the lengthy 'installing ri/rdoc..' stage of gem installation.

However if users do want the ri command they can still access via .ri using pry's shell integration features

@voxik
voxik commented Sep 18, 2012

@banister

[13] pry(main)> .ri save
C:/Users/vita/.pik/rubies/Ruby-193-tcs-20111204/lib/ruby/gems/1.9.1/gems/bundler-1.0.15/lib/bundler/rubygems_integration.rb:143:in `block in replace_gem': rdoc is not part of the bundle. Add it to Gemfile. (Gem::LoadError)
        from C:/Users/vita/.pik/rubies/Ruby-193-tcs-20111204/lib/ruby/gems/1.9.1/bin/ri:18:in `<main>'
Error: there was a problem executing system command: ri save

Cannot say that the result is helpful ;) Interestingly ri command works in the same session.

@ghost Unknown added a commit that referenced this issue Apr 6, 2014
Robert Gleeson Don't try to use the 'less' pager on Windows.
Ref #712.
5afa2b8
@ghost Unknown added a commit that referenced this issue Apr 6, 2014
Robert Gleeson Don't try to run the system pager if 'less' is not available.
Closes #712.
a08aa92
@ghost Unknown added a commit that referenced this issue Apr 6, 2014
Robert Gleeson Platform agnostic.
Ref #712.
f21383a
@ghost Unknown pushed a commit that referenced this issue Apr 6, 2014
@voxik voxik Fix the less check (#712).
The exit status doesn't matter, it is always OK, but an exception is fired only when less is not available on the system.
fc1bb1c
@jazzonmymind jazzonmymind pushed a commit that referenced this issue Nov 23, 2014
rpag Don't try to use the 'less' pager on Windows.
Ref #712.
b18d52a
@jazzonmymind jazzonmymind pushed a commit that referenced this issue Nov 23, 2014
rpag Platform agnostic.
Ref #712.
0665a9f
@jazzonmymind jazzonmymind pushed a commit that referenced this issue Nov 23, 2014
@voxik voxik Fix the less check (#712).
The exit status doesn't matter, it is always OK, but an exception is fired only when less is not available on the system.
40f2155
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.