senny / rvm.el

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

Missing features #38

Closed rejeep closed 10 years ago

rejeep commented 10 years ago

Hi,

I can't solve an issue I have with the current API, but with a combination of some of these I could:

I'm not saying that all or these should be added, it's more a discussion. What do you think?

senny commented 10 years ago

@rejeep I since moved on to rbenv and rbenv.el. Currently I only do bugfixes on rvm.el. If you are interested to work on a PR to extend the functionality, I'd be happy to merge that.

rejeep commented 10 years ago

I can do the work, for sure, but I wanted to discus the features before implementing them. Do they make sense?

senny commented 10 years ago

@rejeep instead of just discussing the API can you name the use-cases where these make sense or are required?

rejeep commented 10 years ago

Definitely,

I want to start an external Ruby server (Sinatra) from Emacs. To do that, I need to switch to that gemset. Currently I do:

(find-file "/path/to/file")
(rvm-activate-corresponding-ruby)
(kill-buffer)
;; start server

So ideal for this case, would be to activate the gemset in a scope and then reset.

senny commented 10 years ago

@rejeep I see. We could extend both rvm-activate-corresponding-ruby and rvm-use to take a function. If given the modification will only take place during that function and then reset. For the path request I suggest to extend the API by one function: rvm-activate-ruby-for or rvm-activate-path. This function will also take the function argument as described above. Further it will use the same logic as rvm-activate-corresponding-ruby but for the given path.

I think your use-case makes perfect sense and these are good additions to rvm.el. Looking forward to a PR.

rejeep commented 10 years ago

I'll think about it for a while and get back with some code :)

rejeep commented 10 years ago

Looking at the source, I'm not sure if I think that rvm-activate-corresponding-ruby and rvm-use should take a callback functions since those are interactive functions. How about only adding rvm-activate-ruby-for:

(defun rvm-activate-ruby-for (path &optional callback)
  "Activate Ruby for PATH.

If CALLBACK is specified, active Ruby for PATH only in that
  function."
  ;; ...
  )

That should be enough, right?

rejeep commented 10 years ago

I implemented the feature here: https://github.com/rejeep/rvm.el/commit/2df533ec5186ddb06533639cba9b03fc4712034e

I don't like the fact that rvm-activate-ruby-for use the interactive function rvm-use. An internal function should be created for that. But since that's how the rest of the source works, I didn't bother.

senny commented 10 years ago

@rejeep agree about the callback. Commit looks good. Can you submit a PR for discussion?

senny commented 10 years ago

Closing in favor of #39