nonsequitur / inf-ruby

218 stars 68 forks source link

Display sent command REPL output in in-place mini popup #148

Closed nfedyashev closed 2 years ago

nfedyashev commented 3 years ago

Sorry for the confusing title, I'm not very familiar with terminology.

It's easier to explain with a gif I could find(although it is emacs-lisp) img

I used this feature quite frequently back when I worked with Clojure(& emacs/CIDER). It's very convenient to just send a specific form and display the immediate results/output right in the same place. In most case one doesn't even need to have REPL session to be visible at that moment.

I can not imagine how difficult would it be to add such feature to inf-ruby. So this issue is just an idea on how to make REPL usage in inf-ruby even more convenient.


Off-topic, @dgutov please consider opening sponsorship for inf-ruby and/or your own Github account. I wanted to donate long time ago but currently there isn't such an option. I'm sure there are other Ruby/emacs users who rely heavily on inf-ruby and want to contribute too.

dgutov commented 3 years ago

Hi!

Any chance you have a link to CIDER's implementation of the "temporary printing" feature? That might speed the effort up a bit.

Overall, I think the feature is less useful in Ruby than it is in Lisp-family of languages, because of ambiguous syntax (what is a sexp?). E.g., if you press C-x C-e after foo.bar.baz, it will complain that baz is an unknown variable or global method, rather than try to evaluate the whole chain. Or evaluate things in the context of the current class/module. So if we work on this feature, I guess improving on all those cases somehow would be the next step.

Off-topic, @dgutov please consider opening sponsorship for inf-ruby and/or your own Github account. I wanted to donate long time ago but currently there isn't such an option. I'm sure there are other Ruby/emacs users who rely heavily on inf-ruby and want to contribute too.

Thanks for the suggestion, I've been considering it. Though TBH I have been mostly thinking of inf-ruby as "done", only requiring superficial maintenance, with other parts of the ecosystem needing more attention.

tunnes commented 2 years ago

Hey guys I think I figure out how to have this behavior on inf-ruby I've used this implementation of Eros package that actually is based on Cider evaluation overlays and this print feature that @dgutov implemented some time ago.

Regarding the following concern, point

if you press C-x C-e after foo.bar.baz, it will complain that baz is an unknown variable or global method, rather than try to evaluate the whole chain.

I couldn't find any way to solve this other than using inf-ruby ruby-send-line feature instead of trying to evaluate only the last expression like on Cider feature.

@dgutov You think that it still makes sense for this feature? So I can open a pull request.

dgutov commented 2 years ago

Hi!

I'd be happy to review such PR.

Regarding ruby-send-sexp, though. ruby-send-line might not be ideal either, e.g. it wouldn't work with chaining.

There might be something mode-specific we can do, though. That would work better with ruby-mode than enh-ruby-mode, at least initially, but people are welcome to improve support for the latter too. Try this command:

(defun ruby-send-last-stmt (&optional print)
  "Send the preceding statement to the inferior Ruby process."
  (interactive "P")
  (let (beg)
    (save-excursion
      (cond
       ((and (derived-mode-p 'ruby-mode)
             (bound-and-true-p smie-rules-function))
        (or (member (nth 2 (smie-backward-sexp ";")) '(";" "#" nil))
            (error "Preceding statement not found"))
        (setq beg (point)))
       (t ; enh-ruby-mode?
        (back-to-indentation)
        (while (and (eq (char-after) ?.)
                    (zerop (forward-line -1)))
          (back-to-indentation))
        (setq beg (point)))))
    (ruby-send-region beg (point)))
  (when print (ruby-print-result)))

Not sure if we should replace the definition of ruby-send-last-sexp with it, but maybe the key binding? It does seem more useful.

tunnes commented 2 years ago

Hey @dgutov, thanks for this suggestion it seems awesome to me 🙌 I'll open a PR as soon as possible so.

About we override the ruby-send-last-sexp key binding I think that seems a good idea, but let me know one thing, the print argument should keep optional to this fn ruby-send-last-stmt? If so how the user could set print as true without overriding the key bind manually calling the fn passing the print argument that we are releasing?

tunnes commented 2 years ago

I mean there is a way to set the optional print argument as true globally in the .emacs.d/init.el without the need to override the default inf-ruby key bindings? 💭

dgutov commented 2 years ago

but let me know one thing, the print argument should keep optional to this fn ruby-send-last-stmt?

I think so, but it could be a user option, I suppose.

Or you can add new command which would serve as a wrapper. I thought you were going to use that approach.

dgutov commented 2 years ago

It would require changing (or setting) a key binding, yes.

Anyway, why don't we see the implementation first, and then discuss the default behavior? Overall, I'm open to moving in the direction of Cider's behavior, for example.

tunnes commented 2 years ago

Or you can add new command which would serve as a wrapper. I thought you were going to use that approach.

Cool I'll add a wrapper so, currently, I was overriding the command in my .emacs.d/init.el with that wrapper.

tunnes commented 2 years ago

Anyway, why don't we see the implementation first, and then discuss the default behavior? Overall, I'm open to moving in the direction of Cider's behavior, for example.

I've agreed I've already finished the implementation, I'm currently writing the pull request description, one second 😬

tunnes commented 2 years ago

@dgutov I've opened the pull request https://github.com/nonsequitur/inf-ruby/pull/153 🔥

dgutov commented 2 years ago

The PR has been merged, so closing.