search

senny / rvm.el

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

rvm-activate-corresponding-ruby gets wrong gemset name #25

Closed tmcgilchrist closed 12 years ago

tmcgilchrist commented 12 years ago

I have rvm-mode setup like so

   (require 'rvm)
   (add-hook 'ruby-mode-hook 'rvm-activate-corresponding-ruby)

I've found that when my .rvmrc file contains comments after rvm use it doesn't correctly pickup the gemset name eg

rvm use --create 1.9.2@project

# Something

It'll try setting the gemset to project#

I assume that the regex "\\([^\"\s]+\\)\\)?\\(?:\"\\|\\)" that is checking for the gemset name is not halting at the newline and is picking up the # from comment.

senny commented 12 years ago

you're right ig rvm.el does not recognize the gemset it's probably the regexp, that does not match. I've added detected for the .rvmrc generated by rvm which is also multiline. I'll look into your specific issue.

tmcgilchrist commented 12 years ago

Maybe changing it to something like "\\([^\"\s\n]+\\)?\\(?:\"\\|\\)" would be better. I wouldn't expect a valid gem set name can include a newline.

I've tried that combination in Emacs RE-Builder and it seems to match on both the rvm generated .rvmrc and my one above.

If you're happy with that I can create a pull request plus tests.

senny commented 12 years ago

if you create a test the modified regexp passes all existing scenarios it's fine for me. Just create a pull request and I'll merge it back.

senny commented 12 years ago

is this still an issue? Please report back if so I'll look into it.

tmcgilchrist commented 12 years ago

Yes this is still an issue.

But unfortunately I haven't found the time to submit a fix plus test for it. Sorry.

senny commented 12 years ago

@tmcgilchrist I'll look into it when I got a few minutes.

senny commented 12 years ago

@tmcgilchrist I pushed a fix, can you update to the latest version and see if it works?

tmcgilchrist commented 12 years ago

Looks good to me. Nice work.