[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 Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen

* Marco Baringer [2008-02-04 17:36+0100] writes:
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:
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:
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:
that's an excellent idea. -- -Marco Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen

* 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:
right, that's why it'd be smart to use slime-current-package.
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 Ring the bells that still can ring. Forget your perfect offering. There is a crack in everything. That's how the light gets in. -Leonard Cohen

* 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:
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:
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:
Which, of course, is not in the REPL buffer.
Considering the amount of time that we spend on this detail, this seems to be an important command, so I fixed it. Helmut.
participants (5)
-
Helmut Eller
-
Madhu
-
Marco Baringer
-
Michael Weber
-
Nikodemus Siivola