Calling slime-inspect at a prompt in the slime repl buffer without any other input will suggest the prompt itself ("CL-USER>" or whatever) as the default value to inspect. I can not think of any case where this behaviour would be useful :)
The attached patch advises slime-sexp-at-point, when the slime-repl.el contrib is loaded, to ignore the prompt text (if point is in the input area, otherwise the normal behaviour of slime-sexp-at-point applies).
Unless anyone objects to the patch (or the implementation) i'll commit this later tomorrow.
Marco Baringer mb@bese.it writes:
Calling slime-inspect at a prompt in the slime repl buffer without any other input will suggest the prompt itself ("CL-USER>" or whatever) as the default value to inspect. I can not think of any case where this behaviour would be useful :)
The attached patch advises slime-sexp-at-point, when the slime-repl.el contrib is loaded, to ignore the prompt text (if point is in the input area, otherwise the normal behaviour of slime-sexp-at-point applies).
Unless anyone objects to the patch (or the implementation) i'll commit this later tomorrow.
I object to the use of defadvice.
Stas Boukarev stassats@gmail.com writes:
I object to the use of defadvice.
why?
- the slime-repl is a contrib and i don't want to modify the "builtin" slime-sexp-at-point to support functionality which may or may not be loaded.
- this late modification of existing functions is exactly what defadvice is supposed to do, the presence of advice is clearly shown by describe-function and it can be disabled if needed.
There are obviously (many) other ways to implement this (buffer local variables, custom flags, redefinition, etc.), but none of them seem as clear, to me at least, as a simple defadvice.
Marco Baringer mb@bese.it writes:
Stas Boukarev stassats@gmail.com writes:
I object to the use of defadvice.
why?
the slime-repl is a contrib and i don't want to modify the "builtin" slime-sexp-at-point to support functionality which may or may not be loaded.
this late modification of existing functions is exactly what defadvice is supposed to do, the presence of advice is clearly shown by describe-function and it can be disabled if needed.
There are obviously (many) other ways to implement this (buffer local variables, custom flags, redefinition, etc.), but none of them seem as clear, to me at least, as a simple defadvice.
It's not apparent when reading the code that it may be redefined somewhere else at some other time. Slime uses variables for cases like this, see `slime-find-buffer-package-function', for example.
Stas Boukarev stassats@gmail.com writes:
It's not apparent when reading the code that it may be redefined somewhere else at some other time.
I don't really think it should be (when reading slime.el), i don't see why the normal slime-inspect function has to have an extra layer of indirection that only the repl code would ever need to use (if there were other places in the code using this variable i would have a different opinion, but there aren't, and i can't see any that would make use of this).
It should of course be apparent when reading the slime-repl.el, but then it is, and the use of defadvice makes it stand out more as a "warning! monkey patching here!" (which it is) rather than just a "oh nothing major, just tweaking a hook" (which it is not).
I see it as an exception to the normal code flow that only applies in certain cases when the slime-repl contrib is loaded, defadvice is the most natural way to express that.
Slime uses variables for cases like this, see `slime-find-buffer-package-function', for example.
yeah, i saw that, and i'm aware of that idiom in slime's code (and in elisp in general). for something like slime-find-buffer-package i can totally see various modes/contribs/languages having different concepts of what a package is and how to find it and wanting to customize that behaviour, that seems like a good hook to have (i have the same opinion about slime-complete-symbol-function and slime-popup-buffer-quit-function), i do not feel the same way about changing slime-sexp-at-point (a sexp is a very well defined thing, you shouldn't be messing with it unless something really weird, like a repl prompt, is going on).
anyway, that's already way too many words, i don't want to have a long drawn out back and forth about it; i will introduce a slime-sexp-at-point-function variable and i'll rewrite the patch to use that.
On Fri, May 10 2013, Marco Baringer wrote:
anyway, that's already way too many words, i don't want to have a long drawn out back and forth about it; i will introduce a slime-sexp-at-point-function variable and i'll rewrite the patch to use that.
It would be easier is to make a different command, say slime-repl-inspect, and bind it to the key where slime-inspect was.
Helmut
-
Helmut Eller -
Marco Baringer -
Stas Boukarev