[slime-devel] slime-repl-set-package should not default the current package when calling slime-read-package-name
hi,
If you're going to change package it doesn't make sense to supply the current package as the default argument, the first thing you're going to do, and you're always going to do this, is delete the package name slime just inserted.
(defun slime-repl-set-package (package) "Set the package of the REPL buffer to PACKAGE." (interactive (list (slime-read-package-name "Package: "))) (with-current-buffer (slime-output-buffer) (let ((unfinished-input (slime-repl-current-input))) (destructuring-bind (name prompt-string) (slime-eval `(swank:set-package ,package)) (setf (slime-lisp-package) name) (setf (slime-lisp-package-prompt-string) prompt-string) (slime-repl-insert-prompt) (insert unfinished-input)))))
* Marco Baringer [2008-02-04 17:36+0100] writes:
hi,
If you're going to change package it doesn't make sense to supply the current package as the default argument, the first thing you're going to do, and you're always going to do this, is delete the package name slime just inserted.
It could make sense if slime-repl-set-package is called in a non-REPL buffer. But that's unlikely, so please commit the change.
Helmut.
* Helmut Eller m2tzkojoae.fsf@common-lisp.net : Wrote on Tue, 05 Feb 2008 00:27:05 +0100:
| * Marco Baringer [2008-02-04 17:36+0100] writes: |> |> If you're going to change package it doesn't make sense to supply the |> current package as the default argument, the first thing you're going |> to do, and you're always going to do this, is delete the package name |> slime just inserted. | | It could make sense if slime-repl-set-package is called in a non-REPL | buffer. But that's unlikely, so please commit the change.
I'd vote against the change. This goes against my use case -- I have only used C-c M-p in buffers having lisp file with (in-package "FOO") forms, to set the REPL package to the current package in the file.
Then I switch to the REPL to enter and eval forms in that package. I preferred this style of interaction.
-- Madhu
On 2/5/08, Madhu enometh@meer.net wrote:
I'd vote against the change. This goes against my use case -- I have only used C-c M-p in buffers having lisp file with (in-package "FOO") forms, to set the REPL package to the current package in the file.
Then I switch to the REPL to enter and eval forms in that package. I preferred this style of interaction.
How about a slime-shortcut (R, maybe?) that goes to repl, and sets the repl to the package of the last buffer?
Cheers,
-- Nikodemus
On Feb 5, 2008, at 00:27 , Helmut Eller wrote:
If you're going to change package it doesn't make sense to supply the current package as the default argument, the first thing you're going to do, and you're always going to do this, is delete the package name slime just inserted.
It could make sense if slime-repl-set-package is called in a non-REPL buffer. But that's unlikely, so please commit the change.
I do this all the time. How about checking whether the current REPL package is the same as the default argument, in which case the default will be empty?
Cheers, Michael
Michael Weber michaelw+slime@foldr.org writes:
I do this all the time. How about checking whether the current REPL package is the same as the default argument, in which case the default will be empty?
that's an excellent idea.
* Marco Baringer 87fxw7c08k.fsf@arsenic.bese.it : Wrote on Tue, 05 Feb 2008 08:48:27 +0100:
| Michael Weber michaelw+slime@foldr.org writes: | |> I do this all the time. How about checking whether the current REPL |> package is the same as the default argument, in which case the default |> will be empty? | | that's an excellent idea.
If you are going to change this function, you should note that currently `slime-repl-set-package' ends up calling `slime-search-buffer-package' to fill in the default, which would be the wrong function to call if `slime-repl-set-package' is invoked in the REPL, as it would always return `nil' -- thereby leaving an empty default.
[Also, the behaviour of `slime-search-buffer-package' is what makes it useful in invoking `slime-repl-set-package' in a file looking at a lisp buffer, and is dependent on where the cursor is -- so a repl shortcut would probably not cut it. My *slime-scratch* file for instance has several cl:in-package forms]
-- Madhu
Madhu enometh@meer.net writes:
- Marco Baringer 87fxw7c08k.fsf@arsenic.bese.it :
Wrote on Tue, 05 Feb 2008 08:48:27 +0100:
| Michael Weber michaelw+slime@foldr.org writes: | |> I do this all the time. How about checking whether the current REPL |> package is the same as the default argument, in which case the default |> will be empty? | | that's an excellent idea.
If you are going to change this function, you should note that currently `slime-repl-set-package' ends up calling `slime-search-buffer-package' to fill in the default, which would be the wrong function to call if `slime-repl-set-package' is invoked in the REPL, as it would always return `nil' -- thereby leaving an empty default.
right, that's why it'd be smart to use slime-current-package.
[Also, the behaviour of `slime-search-buffer-package' is what makes it useful in invoking `slime-repl-set-package' in a file looking at a lisp buffer, and is dependent on where the cursor is -- so a repl shortcut would probably not cut it. My *slime-scratch* file for instance has several cl:in-package forms]
i have no idea what you're talking about here. slime-repl-set-package, which is also a shortcut function, uses slime-pretty-find-buffer-package (it won't for much langer but the functionality remains the same). slime-pretty-find-buffer-package, so therefore slime-repl-set-package, knows how to deal with multiple in-package forms.
* Marco Baringer 878x1zsji9.fsf@arsenic.bese.it : Wrote on Tue, 05 Feb 2008 12:57:50 +0100: |> If you are going to change this function, you should note that currently |> `slime-repl-set-package' ends up calling `slime-search-buffer-package' |> to fill in the default, which would be the wrong function to call if |> `slime-repl-set-package' is invoked in the REPL, as it would always |> return `nil' -- thereby leaving an empty default. | | right, that's why it'd be smart to use slime-current-package.
I decided not to update now so I won't see what you have done. However if you followed the call chain:
slime-pretty-find-buffer-package -> slime-find-buffer-package
it should have been clear that what ought to have changed is the value of the variable `slime-find-buffer-package-function' which was already factored out for customization.
|> [Also, the behaviour of `slime-search-buffer-package' is what makes it |> useful in invoking `slime-repl-set-package' in a file looking at a lisp |> buffer, and is dependent on where the cursor is -- so a repl shortcut |> would probably not cut it. My *slime-scratch* file for instance has |> several cl:in-package forms] | | i have no idea what you're talking about here.
This was in response to another suggestion from Nikodemus in this thread http://permalink.gmane.org/gmane.lisp.slime.devel/7021
About why the proposed `R' repl shortcut may not work very well.
| slime-repl-set-package, which is also a shortcut function, uses | slime-pretty-find-buffer-package (it won't for much langer but the | functionality remains the same).
Again, this would have been unnecessary if you had followed the call chain above.
| slime-pretty-find-buffer-package, so | therefore slime-repl-set-package, knows how to deal with multiple | in-package forms.
-- Madhu
On 2/6/08, Madhu enometh@meer.net wrote:
This was in response to another suggestion from Nikodemus in this thread http://permalink.gmane.org/gmane.lisp.slime.devel/7021
About why the proposed `R' repl shortcut may not work very well.
Oops, that was bad terminology on my part. I did not mean a REPL shortcut of the ",foo" variety, but a slime-selector method.
Like so:
(def-slime-selector-method ?R "Slime Read-Eval-Print-Loop using current package." (slime-repl-set-package (slime-find-buffer-package)) (slime-output-buffer))
Cheers,
-- Nikodemus
* "Nikodemus Siivola" Wrote on Wed, 6 Feb 2008 11:39:12 +0200: |> About why the proposed `R' repl shortcut may not work very well. | | Oops, that was bad terminology on my part. I did not mean a REPL | shortcut of the ",foo" variety, but a slime-selector method. | | Like so: | | (def-slime-selector-method ?R | "Slime Read-Eval-Print-Loop using current package." | (slime-repl-set-package (slime-find-buffer-package)) | (slime-output-buffer))
Ah, that works very well. It would be nice to see this go in. [I don't believe I've not bound slime-selector to a key yet, but I cant seem to decide on a good one-keystroke binding! ]
-- Madhu
The following 2 changes contradict each other,
* 2007-11-24 Helmut Eller heller@common-lisp.net | * slime.el (slime-search-buffer-package): Don't remove double | quotes or "#:", swank:parse-package takes care of that.
* 2008-02-05 Marco Baringer mb@bese.it | (slime-repl-set-package): Only prompt with a default package if | the repl's package is different from the current package.
in a lisp file with an (in-package "FOO") form.
(slime-current-buffer) returns the quoted string which is compared to the unquoted string returned by (slime-lisp-package) and defeats Marco's intent [expressed in the 2nd changelog entry]
-- Madhu
* Madhu [2008-02-23 08:49+0100] writes:
The following 2 changes contradict each other,
- 2007-11-24 Helmut Eller heller@common-lisp.net
| * slime.el (slime-search-buffer-package): Don't remove double | quotes or "#:", swank:parse-package takes care of that.
- 2008-02-05 Marco Baringer mb@bese.it
| (slime-repl-set-package): Only prompt with a default package if | the repl's package is different from the current package.
in a lisp file with an (in-package "FOO") form.
(slime-current-buffer) returns the quoted string which is compared to the unquoted string returned by (slime-lisp-package) and defeats Marco's intent [expressed in the 2nd changelog entry]
In the REPL buffer (slime-current-buffer) returns (slime-lisp-package). That's the primary situation where the proposed default would be wrong. There a few more such situations but it's hardly worth to detect them all.
Helmut.
* Helmut Eller Wrote on Sat, 23 Feb 2008 11:20:39 +0100:
| * Madhu [2008-02-23 08:49+0100] writes: |> The following 2 changes contradict each other, |> |> * 2007-11-24 Helmut Eller heller@common-lisp.net |> | * slime.el (slime-search-buffer-package): Don't remove double |> | quotes or "#:", swank:parse-package takes care of that. |> |> * 2008-02-05 Marco Baringer mb@bese.it |> | (slime-repl-set-package): Only prompt with a default package if |> | the repl's package is different from the current package. |> |> in a lisp file with an (in-package "FOO") form. |> |> (slime-current-buffer) returns the quoted string which is compared to |> the unquoted string returned by (slime-lisp-package) and defeats Marco's |> intent [expressed in the 2nd changelog entry] | | In the REPL buffer (slime-current-buffer) returns (slime-lisp-package). | That's the primary situation where the proposed default would be wrong.
I'm not sure I understand. The situation in my bug report is about visiting a file with a:
(in-package "FOO")
form.
Inside this buffer, calling C-c M-p (i.e. slime-repl-set-package) will prompt you with package FOO even if the repl's current package is FOO.
The point of Marco's patch was to not fill in the prompt when the current package is identical to the package in the repl.
-- Madhu
| There a few more such situations but it's hardly worth to detect them | all.
* Madhu [2008-02-23 12:54+0100] writes:
| In the REPL buffer (slime-current-buffer) returns (slime-lisp-package). | That's the primary situation where the proposed default would be wrong.
I'm not sure I understand. The situation in my bug report is about visiting a file with a:
(in-package "FOO")form.
Which, of course, is not in the REPL buffer.
Inside this buffer, calling C-c M-p (i.e. slime-repl-set-package) will prompt you with package FOO even if the repl's current package is FOO.
The point of Marco's patch was to not fill in the prompt when the current package is identical to the package in the repl.
Considering the amount of time that we spend on this detail, this seems to be an important command, so I fixed it.
Helmut.
-
Helmut Eller -
Madhu -
Marco Baringer -
Michael Weber -
Nikodemus Siivola