search

senny / rvm.el

use rvm to manage ruby versions within emacs
214 stars 42 forks source link

strip comments from .rvmrc file before parsing version #31

Closed skaes closed 11 years ago

skaes commented 11 years ago

My .rvmrc files frequently contain comments which leads to incorrect gemset names.

This commit fixes the problem for me.

senny commented 11 years ago

great! Do you think you could add a test-case so that we don't run into regressions with this problem? You can find the tests here: https://github.com/senny/rvm.el/blob/master/tests/rvm-unit-tests.el

skaes commented 11 years ago

I'd be happy to add a test case, If I know how to run the tests ...

On Sat, Mar 30, 2013 at 11:33 AM, Yves Senn notifications@github.comwrote:

great! Do you think you could add a test-case so that we don't run into regressions with this problem? You can find the tests here: https://github.com/senny/rvm.el/blob/master/tests/rvm-unit-tests.el

— Reply to this email directly or view it on GitHubhttps://github.com/senny/rvm.el/pull/31#issuecomment-15672497 .

senny commented 11 years ago

You can run them using this script: https://github.com/senny/rvm.el/blob/master/tests/rvm-tests

skaes commented 11 years ago

Added a test. However, the test only works when tests/rvm-tests is invoked from the top level project directory.

Didn't know how to make this more robust with emacs-lisp.

On Sat, Mar 30, 2013 at 1:05 PM, Yves Senn notifications@github.com wrote:

You can run them using this script: https://github.com/senny/rvm.el/blob/master/tests/rvm-tests

— Reply to this email directly or view it on GitHubhttps://github.com/senny/rvm.el/pull/31#issuecomment-15673536 .

senny commented 11 years ago

I think it would be good if you could move the code inside the rvm--rvmrc-parse-version function. This way you can also simplify the test by inlining the contents of the .rvmrc and testing against rvm--rvmrc-parse-version as the other tests do. What do you think?

skaes commented 11 years ago

Go for it. ;)

On Sat, Mar 30, 2013 at 5:23 PM, Yves Senn notifications@github.com wrote:

I think it would be good if you could move the code inside the rvm--rvmrc-parse-version function. This way you can also simplify the test by inlining the contents of the .rvmrc and testing against rvm--rvmrc-parse-version as the other tests do. What do you think?

— Reply to this email directly or view it on GitHubhttps://github.com/senny/rvm.el/pull/31#issuecomment-15677133 .

senny commented 11 years ago

@skaes can you check if https://github.com/senny/rvm.el/pull/33 works for you?

senny commented 11 years ago

Thanks @skaes for your contribution :heart:

I'm closing this one in favor of #33