Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rvm list known doesn't work with PAGER="less -RM" #899

Closed
hoelzro opened this issue Apr 10, 2012 · 12 comments
Closed

rvm list known doesn't work with PAGER="less -RM" #899

hoelzro opened this issue Apr 10, 2012 · 12 comments
Assignees
Milestone

Comments

@hoelzro
Copy link

hoelzro commented Apr 10, 2012

Just what it sounds like.

[11:34:23] rhoelz@Robs-MacBook-Pro.local ~ $ rvm list known
/Users/rhoelz/.rvm/scripts/functions/utility: line 382: less -RM: command not found
@ghost ghost assigned mpapis Apr 10, 2012
@ghost
Copy link

ghost commented Apr 10, 2012

@hoelzro Please provide a gist of

rvm --trace list known

I am on a Macbook Pro as well and it works fine here, so something is going on over there, as shown by

∴ rvm list known | gist
https://gist.github.com/2353264

@hoelzro
Copy link
Author

hoelzro commented Apr 10, 2012

@deryldoucette Did you try it with PAGER set to 'less -RM', like the subject says?

@ghost
Copy link

ghost commented Apr 10, 2012

Actually no, because you don't set params to the pager in PAGER. If you must use params, you make an alias like so

export PAGER='less'
alias less='less -RM'

The PAGER variable is only for defining the binary to use, not any additional params. You do that with an alias, as shown above.

@ghost
Copy link

ghost commented Apr 10, 2012

@mpapis not sure there is a real need to be able to have RVM violate the unix premise that PAGER should only be the binary name and not include params. However, the reason it fails under RVM is due to

${_pager[@]}" "$@"

(Line 382 of $rvm_path/scripts/utility) which causes it to be evaluated as a full binary name.

@hoelzro
Copy link
Author

hoelzro commented Apr 10, 2012

Where is this premise documented that PAGER must be an executable name? What other program(s) follow this premise and fail to operate when PAGER is set to somethine like 'less -RM'?

@mpapis
Copy link
Member

mpapis commented Apr 11, 2012

we have two problems here, and I think the best solution to it is an array:

PAGER=( less -RM )

OR:

PAGER=( "/path with spaces/ to my awesome pager" )

but I'm not sure how this will work with other software as basically before OSX [[:space:]] was not considered a good idea to put in paths, but now with OSX - you can get spaces everywhere, which makes it a lot harder to handle such use cases.

@ghost
Copy link

ghost commented Apr 11, 2012

@hoelzro That has been a unix basic tenant since I was taught unix in the early 80s, and was taught to me by both commercial and open source communities involved with unix since their inception! You don't have to agree with it, just know that its been so. It wasn't until the advent of Linux that we started finding this changing, and Linux is not the end all be-all of Unix. The 'premise', to use your word, to not include params in core (PAGER is one of the oldest OS variables I know of) vars is for compatibility, and to use shell aliases or batch files to override. The reasoning for this is demonstrated by your current issue. Since a parameter which describes the binary to use, and demonstrated by @mpapis 's common use pattern, can cause compatibility issues and behaviour as you're showing, this is why shell aliases and script overrides are the normal methods for overriding.

As @mpapis also has shown, spaces were not used because the OS didn't know how to handle them, and even today chokes on them quite often even though we've has spaces in file and directory names for a long time now. Adding parameters in the vars like that required the use of the very thing that caused issues... spaces.

As I said, you don't have to agree with that 'premise', just know this is where it comes from, and is still used (in mixed mode and with mixed results) today.

@ghost
Copy link

ghost commented Apr 11, 2012

@mpapis btw, I agree with the change you just proposed.

@hoelzro
Copy link
Author

hoelzro commented Apr 11, 2012

@deryldoucette You make a good argument. I don't fully agree (I don't see a problem with allowing this functionality), but it's not my project and now I understand where you're coming from.

@FooBarWidget
Copy link

I just wanted to report the same issue because I have $PAGER set to less -R. I didn't know $PAGER is supposed to be an executable name.

Anyway, the issue is easily solved locally through a wrapper script /usr/local/bin/less-r:

#!/bin/sh
exec less -R "$@"

and export PAGER=less-r

@mpapis
Copy link
Member

mpapis commented Apr 17, 2012

@FooBarWidget the problem is that PAGER can be anything and we need to handle properly both less -R and /path with spaces/less. Although the second use case is less likely to happen we still need a way to handle it.

mpapis added a commit that referenced this issue Apr 17, 2012
@mpapis
Copy link
Member

mpapis commented Apr 17, 2012

this should temporally fix it, but rescheduling to rvm2 for full fix

@mpapis mpapis closed this as completed May 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants