search

minad / consult

:mag: consult.el - Consulting completing-read
GNU General Public License v3.0
1.12k stars 98 forks source link

consult-register: Set register-command-info (Emacs 30) #916

Open slotThe opened 4 months ago

slotThe commented 4 months ago

Emacs recently changed how register jumping/loading is handled. The variable register-use-preview may be used to adjust that behaviour; it's read and acted upon by (e.g.) register-read-with-preview. That function (which we call inside of consult-register-load) checks which command called it and gets some information about that via the register-command-info defmethod. If (say) the command indicates that it just wants to jump to some register, an additional confirmation is skipped when register-use-preview is set to an appropriate value.

Thus, to completely restore how Emacs used to handle register, we need consult-register-load to signal that all it does it jump to the register.

N.b.: I didn't change the changelog yet, as I don't know if this qualifies as something that should be in there.

minad commented 4 months ago

Thanks for this. I noticed the debates regarding the register changes. I am not sure if everything is settled regarding that matter?

For now I won't merge the change. I will do so after I had time to test the new register behavior and the interaction with Consult. This will likely happen when the emacs-30 branch is created. Do you have an idea when that might happen? Then I will also bump Compat to compat-30 and finalize Emacs 30 support in my packages.

slotThe commented 4 months ago

Thanks for this. I noticed the debates regarding the register changes. I am not sure if everything is settled regarding that matter?

Yeah it was quite a heated discussion, but people seem to have cooled off a bit now and this commit is—to my knowledge—the end of that story. There is actually another option to register-use-preview now (which I missed at first): traditional. That one does not require downstream packages to change and essentially restores the old Emacs 29 functionality. However, the register-command-info is still used for other values of register-use-preview. Some of these, like a value of nil (how descriptive), add some eye-candy and so I think downstream packages should still adjust and support that use-case.

For now I won't merge the change. I will do so after I had time to test the new register behavior and the interaction with Consult. This will likely happen when the emacs-30 branch is created. Do you have an idea when that might happen? Then I will also bump Compat to compat-30 and finalize Emacs 30 support in my packages.

Okay, fair enough, thanks for considering! I have no idea when emacs-30 is going to be cut, but I'd imagine that it'll take a few more months.

minad commented 11 hours ago

I've just migrated to Emacs 30. Is this PR still needed? I don't see any change in behavior with register-use-preview=nil.

slotThe commented 3 hours ago

Is this PR still needed?

I believe so. Without this change, even setting register-use-preview=nil, one has to give Emacs an additional confirmation when using consult-register-load. E.g., given a register on a, with this change (and before Emacs 30) one would just have to execute consult-register-load a to jump to the register, while on Emacs 30 without this change it's now consult-register-load a RET.

minad commented 1 hour ago

It seems to me that the change is unnecessary. One can simply set register-use-preview to traditional (the default) and everything keeps working as is. What is the advantage of setting register-use-preview to never or nil?