joaotavora / sly

Sylvester the Cat's Common Lisp IDE
1.23k stars 139 forks source link

sly-remove-method #586

Closed charJe closed 1 year ago

charJe commented 1 year ago

Named after cl:remove-method. Similar to sly-undefine-function. It would remove the method at point with prompting for the qualifiers and specifiers as arguments to find-method. It would also fill in all those prompts with default values of the method defined at point.

I can work on if you want; I just want to flesh out the idea first. What do you think.

joaotavora commented 1 year ago

charJe @.***> writes:

Named after cl:remove-method. Similar to sly-undefine-function. It would remove the method at point with prompting for the qualifiers and specifiers as arguments to find-method. It would also fill in all those prompts with default values of the method defined at point.

I can work on if you want; I just want to flesh out the idea first. What do you think.

I think it's a good idea, maybe even great :-) Even better if it would somehow offer completion of methods of a given generic function that aren't at point.

I remember working on vaguely similar stuff with the trace functionality and finding out method signatures portably wasn't easy. But maybe you'll find it easier, or fix that.

Let's see a pull request, and then we can iterate from there.

João

charJe commented 1 year ago

I was originally just thinking 3 prompts: 1) method name 2) qualifiers 3) specializers. Both 2 and 3 would be typed (or auto-filled) as a list qualifiers: (:around) as an example.

offer completion of methods of a given generic function that aren't at point

What would the user interface look like for this? The only thing I can think of is a new prompt for each specializer. I could see that being unergonomic from pressing enter so many times when all the specializers are auto-filled correctly from the method at point. Also there would be no way to go back and change a previously entered specializer.

What do you think the behavior should be in the case that no method is found? On one hand, removing a non-existent method is a no-op that can be considered a success; on the other hand, I might want to know if I typed a specializer incorrectly.

joaotavora commented 1 year ago

I think only 2 prompts, is better. Else it becomes hard to navigate. Keep in mind in Emacs going back prompts is hard or impossible. So as soon as you commit an answer, you have to stick with it. So please make it 2 prompts.

offer completion of methods of a given generic function that aren't at point What would the user interface look like for this?

Simple, just

(defun sly-remove-method (method) "Remove a method"
   (interactive (sly--read-method)) ....

In sly--read-method you present the two prompts, one for the generic, the other for the method within the generic, with completion. At each prompt, you check if there's something which looks like an answer to that prompt around point and you offer it as the default value for the prompt. So for example if point is inside the second method:

(defmethod fooey :around ((bar baz) quux quuz) ...)
(defmethod fooey :before (bar quux quuz) ...)
(defmethod barey (bar quuz) ...)

Then M-x sly-remove-method would first ask

[sly] Remove method from which generic (default `FOOEY')?

And completion would offer FOOEY and BAREY. require-match would be t, by the way.

After you make that choice, it asks you

[sly] Remove which method from FOOEY (default `(FOOEY :BEFORE (t t t))')?

And completion would offer you the two methods.

You have to come up with a suitable format for sly--read-method to return, but that's up to you. It has to:

joaotavora commented 1 year ago

What do you think the behavior should be in the case that no method is found? On one hand, removing a non-existent method is a no-op that can be considered a success; on the other hand, I might want to know if I typed a specializer incorrectly.

If you design completion correctly, the chances of that happening are very reduced or even eliminated.

But if you do find a way to give some bogus signature to sly-remove-method (perhaps by calling it directly). You should get a Lisp error, boom SLY-DB in your face. This is how other parts of SLY work already, so don't worry about it.

tdrhq commented 1 year ago

Btw, I think the completion of methods seems excessive, considering that sly-inspect already provides a UI to list and remove methods:

Screenshot from 2023-04-24 01-14-25

It's a little convoluted to get to it though, admittedly. You have to inspect a method, then click the generic function associated with it, and then you get this page.

charJe commented 1 year ago

😄 The more you know! Are you saying that the existence of completion for methods is excessive or just the way I did it? Perhaps I could use the same (already existing) functionality as the generic function inspector.

joaotavora commented 1 year ago

It's true there are now two UI ways to remove a method, but I don't think it's excessive at all. In particular, it's not easy in the Inspector method to remove the method "at point".

In fact @charJe 's implementation in #588 (where this discussion should be proceeding, btw) is like this precisely because I asked for that model. I still haven't tested it though (but it looks very good).

tdrhq commented 1 year ago

@joaotavora Okay, just wanted to make sure this way of removing methods wasn't something @charJe overlooked since it's kind of hidden away. I've definitely had a hard time in the past figuring out how to get to this page each time I had to to do it.

joaotavora commented 1 year ago

Implemented in #588.