Remove never mind from prompt.

[theothornhill]

This PR removes the never mind option in tmm when using code action. The prompt already clearly states that C-g or ESC ESC ESC is enough to cancel, so I think never mind option only adds noise.

Yet again mostly a suggestion fix :)

[nemethf]

I'm not sure what was the original intent, but if there's only one code action, the current implementation allows the user to cancel applying the code action by selecting "never mind". On the other hand, if I remember correctly, the tmm menu is not shown if there's only one candidate, and therefore it is not possible to press C-g.

[joaotavora]

Yes, I'd like hard proof that the "never mind..." isn't doing anything there. I admit I cannot remeber why I added it (maybe vc-region-history has clues), but the João from 2 years ago wasn't completely clueless, so there might have been a reason.

[theothornhill]

I’ll look into it a little more! While I’m here - would an acceptable solution be skipping tmm altogether and use something like completing-read? It would then supply options to ivy, helm, Ido etc.

Now I get ivy completion when I press tab after tmm prompt is shown, but entering something there would just enter value into minibuffer, not execute choice

[theothornhill]

It looks like never-mind was added to the initial commit of that function, so I cannot find any clues there. My humble guess is that this value is there for mouse clicking. I don't think you can cancel the prompt only with mouse unless you get that option. However C-g is pretty ubiquitous still, so maybe ok anyways?

[joaotavora]

I think the option was added for the case of only one option, as @nemethf suggests. Doesn't seem to be there for mouse-clicking, since it's in the else branch of the preceding if, which checks if x-popup-menu should be used (and that's the mouse).

You can try your hand at a completing-read solution, but completing-read is for completing strings so be warned that it's tricky to use that function to get at the associated value of an association. I had that problem many moons ago in yasnippet and it required more code than should be needed.

[theothornhill]

I see. So it is intended behaviour not to auto-action when only one action is offered? It feels like the behaviour I would want is for it to execute, trusting server to do the correct thing if it is unambiguous

I see three approaches:

1. Killing this PR completely, keeping old behaviour
2. Using this PR, enabling auto execution of code action when only one action exists
3. Adding a defcustom so that we get a choice.

I guess the third option is the least destructive to other peoples workflows, (yet improving on mine :) )

Regarding completing-read, it seems like lsp-mode uses a wrapper for that in their code, and has similar functionality to this, so maybe I can get some inspiration from there. I'll have a look

[joaotavora]

So it is intended behaviour not to auto-action when only one action is offered?

Yes.

I see three approaches:

I'd like to avoid 3, if I can. 2 is possible, maybe adding an informative message.

1 doesn't seem so bad tbh. Would you rather have a y-or-n-p prompt?

Regarding completing-read, it seems like lsp-mode uses a wrapper for that in their code, and has similar functionality to this, so maybe I can get some inspiration from there. I'll have a look

If you come up with a good wrapper for this, a good place to put it is in Emacs itself (though it can start out its life in Eglot).

[theothornhill]

1 doesn't seem so bad tbh. Would you rather have a y-or-n-p prompt?

The reason I want this change is that when I do some refactoring, I like to use LSPs help with for example imports. If there are a lot of imports that needs to update, or if I add a method to the file, it's a lot of steps before server does anything. My last PR (defcustom confirm-server-initiated-edits) fixes one of these, but for all of these small imports I'd like not to confirm each time. It's not terrible, by all means. Just feels a tiny bit too repetetive. Also, at work, I need to use Windows, which has super slow IO, so getting these prompts take a few seconds each time

If you come up with a good wrapper for this, a good place to put it is in Emacs itself (though it can start out its life in Eglot).

I will think about this

[joaotavora]

If confirm-server-initiated-edits was called confirm-server-initiated-actions, I guess we could use that for both cases, right?

Also, at work, I need to use Windows, which has super slow IO, so getting these prompts take a few seconds each time

Been there. But it's usually much worse with launching processes, not particularly with sending things through stdin/stdout, in my experience. If your LSP server supports it, though, communicating via network sockets might be much, much faster (Common Lisp's SLY/SLIME use that technique very effectively).

[theothornhill]

Made changes to the defcustom and wrapped the thing in another if.

What do you think?

If your LSP server supports it, though, communicating via network sockets might be much, much faster (Common Lisp's SLY/SLIME use that technique very effectively).

I didn't get this working in typescript-language-server unfortunately.

I'll look into getting FSF agreement. Do I just mail fsf?

[joaotavora]

Too many unsolicited whitespace changes. Also in the other PR. If you want to do this cleanup, open a separate PR.

[joaotavora]

I'll look into getting FSF agreement. Do I just mail fsf?

Maybe that works, too, but I've seen people just mail [email protected] asking for the copyright assignment form. It's not considered spam (I think) and it hints at who's joining the team. They'll email it back to you off-list.

[theothornhill]

Too many unsolicited whitespace changes. Also in the other PR. If you want to do this cleanup, open a separate PR.

Sorry - wasn't meant to be added. Fixed!

[nemethf]

I'm sorry to butt in, but I think it would be a bit better UI, if a message with the title of the action was displayed to the user in the case of auto-execution and a single action item. That way the users would receive an extra info helping them to decide whether they want to undo the action or not. I guess most of the time they don't want to undo, so this would indeed result in a simpler workflow.

Additionally, isn't a define-obsolete-variable-alias necessary?

[joaotavora]

Additionally, isn't a define-obsolete-variable-alias necessary?

Not necessarily in Eglot, and not necessarily for changing the name of such a new variable, I think.

[joaotavora]

Regarding the rest of your comment, yes it makes sense. @theothornhill I haven't checked the code yet, but I'd like you to add @nemethf's suggestion regardless.

[theothornhill]

I'm sorry to butt in

I'm glad you did!

I think it would be a bit better UI, if a message with the title of the action was displayed to the user in the case of auto-execution and a single action item.

I agree completely and will make an effort to fix this :)

[theothornhill]

I'm closing this in favour of my next PR