Do not install a global advice on tabulated-list-revert #593
Conversation
Instead define our own revert function for embark-collect-mode buffers. It is good practice to avoid advices, and global advices even more so.
|
That does look a little better, thanks! |
|
You know, that entire hook and revert function could probably be removed now. I doubt anyone still uses them. It was useful back in the dark days when embark also had a completions UI. |
|
On 2/3/23 00:28, Omar Antolín Camarena wrote:
You know, that entire hook and revert function could probably be removed
now. I doubt anyone still uses them. It was useful back in the dark days
when embark also had a completions UI.
Okay, please do that then. All simplifications are welcome in my eyes!
When I applied the change I didn't dig deeper, it was just a mechanic
change.
|
|
In fact, if I recall, correctly I added it so Prot could have an easy way of resize the embark completions window to the minimum height needed for the candidates. :) |
|
On 2/3/23 00:37, Omar Antolín Camarena wrote:
In fact, if I recall, correctly I added it so Prot could have an easy
way of resize the embark completions window to the minimum height needed
for the candidates. :)
There seems to be still a usage in the embark-collect-zebra-minor-mode.
But there you could use as well the tabulated-list-revert-hook? Why do
we need an extra hook?
|
|
It's an extra hook so that it is not shared with non-embark tabulated lists, i.e., that way users can set it to a hook run whenever you revert an embark collect buffer but not when you revert some other tabulated list. For the purposes of the zebra-mode that is not important because the hook it adds is added locally any way. |
|
Sure, I understand. But it is still almost useless since you can add a hook to |
|
Yeah, I should remove it. And also fix zebra-mode! It uses the revert hook, because embark-live used to use revert, but since the new lighter embark-live came about it doesn't use reverting anymore. This means that zebra-mode doesn't actually update in embark-live buffers. |
|
Btw, zebra mode should be rewritten anyway to not use overlays, which are too expensive for this use case. It would be better to use text properties. Do you still use zebra mode? It is nice because of the funny name, but I would also not mind if it is removed or demoted to the wiki. :-P |
|
It is still the only sensible way to browse the kill-ring. Not that I actually browse the kill-ring that much. |
|
I personally wouldn't miss zebra mode if it were gone. |
|
I can live with 0 sensible ways to browse the kill-ring, I guess. 🤣 |
|
I'll carefully clean these things up in the next few days. |
|
And then release immediately! :) |
Just opening this such that the discussion in oantolin#593 does not get lost. Remove the zebra and the (almost useless) embark-collect-revert-hook. Given how much code space the Zebra needs, I don't think it is justified to keep it in captivity in our zoo. As alternative I suggest a meerkat mode.
Just opening this such that the discussion in oantolin#593 does not get lost. Remove the zebra and the (almost useless) embark-collect-revert-hook. Given how much code space the Zebra needs, I don't think it is justified to keep it in captivity in our zoo. As alternative I suggest a meerkat mode.
Instead define our own revert function for embark-collect-mode buffers. It is good practice to avoid advices, and global advices even more so.
(I did some advice cleaning in my Vertico and Corfu packages and then I also found this advice in Embark.)