slime / slime

The Superior Lisp Interaction Mode for Emacs
http://common-lisp.net/project/slime/
1.9k stars 336 forks source link

Completion for `slime-describe-symbol` and other functions #303

Open xiongtx opened 8 years ago

xiongtx commented 8 years ago

I took a look at the completion code for SLIME, and I'm not sure why it uses read-from-minibuffer rather than completing-read. I'm guessing that it's because completing-read is a relative latecomer to Emacs Lisp (I'm not sure that it is; just taking a stab in the dark here)?

read-from-minibuffer does not, as far as I can tell, have built-in completion capabilities, which is why we have to hack in the slime-setup-completion in the slime-minibuffer-setup-hook.

Benefits of using completing-read include default integration with Helm. You can see this with the common-lisp-hyperspec-glossary-term command. I find Helm's incremental completion much better than the built-in completion-at-point that you can use in the minibuffer, although I believe completion-at-point also works with completing-read.

As a quick example, try the following for yourself (I use slime-simple-completions for...simplicity):

(defun *-slime-read-from-minibuffer (prompt &optional initial-value history)
  "Completing-read a string from the minibuffer, prompting with prompt."
  (interactive)
  (let ((minibuffer-setup-hook (slime-minibuffer-setup-hook)))
    (completing-read prompt (slime-simple-completions (or initial-value ""))
             nil nil nil
             (or history 'slime-minibuffer-history))))

(defun *-slime-read-symbol-name (prompt &optional query)
  "Either read a symbol name or choose the one at point.
The user is prompted if a prefix argument is in effect, if there is no
symbol at point, or if QUERY is non-nil."
  (cond ((or current-prefix-arg query (not (slime-symbol-at-point)))
         (*-slime-read-from-minibuffer prompt (slime-symbol-at-point)))
        (t (slime-symbol-at-point))))

(defun *-slime-describe-symbol (symbol-name)
  "Describe the symbol at point."
  (interactive (list (*-slime-read-symbol-name "Describe symbol: ")))
  (when (not symbol-name)
    (error "No symbol given"))
  (slime-eval-describe `(swank:describe-symbol ,symbol-name)))
luismbo commented 8 years ago

Nice! I particularly like the Helm integration bit! Do you have a pull request on the way? :-)

I don't know why read-from-minibuffer is being used. Git blame shows a bunch of folks working on that code who, like me, are Common Lisp programmers first and Elisp programmers second, out of necessity. Perhaps that's why. ;-)

xiongtx commented 8 years ago

I'll look into the code some more before making a pull request; the completion functionality is used for many things (including, it seems, files), so I don't want to break anything.

Update: looking at the code, it appears that slime-read-from-minibuffer does something very interesting: it sets the context for completion-at-point, then relies on that function to do the completion for read-from-minibuffer (made possible by hijacking the minibuffer keymap) instead of providing a completion table to completing-read.

From what I can see, the same completion mechanism is used for everthing in SLIME, from reading minibuffer input to completing at point in a "normal" buffer.

This makes slime-read-from-minibuffer a rather complex function, since it is used for many different purposes. Ideally, we'd have different functions for reading in file names, CL symbols, etc., but what we have is a single God function that does everything (and I'm not sure that it's even correct in all cases!).

Therefore, I'm hesitant to make any changes. If this problem annoys me enough, I'll reinvest some effort into it, but right now it seems that the benefits do not outweigh the risks.