search

DarwinAwardWinner / amx

An alternative M-x interface for Emacs.
GNU General Public License v3.0
151 stars 9 forks source link

`amx` does not work well with certain commands #48

Closed ywwry66 closed 1 year ago

ywwry66 commented 2 years ago

Hi, I am using amx on top of ivy. I found that amp does not work well with certain commands. For example, after invoking M-x maxima, I can see that in ~\.emacs.d\amx-item, maxima is recorded in amx-history and its count in amx-data is added one. However, if I restart emacs, the maxima entry in amx-history is just gone, while I would expect it to stay there being the most recent command used. Am I missing something or is this a bug? Thanks.

bestlem commented 2 years ago

I get the same with a command I made `gnus-mwb' it is not remembered after a startup whilst gnus is trated correctly.

ywwry66 commented 2 years ago

I just did some research myself and figured out the root of the problem. Previously I had something like this in my init file:

(use-package amx
  :config
  (amx-mode 1))

(use-package maxima
  :ensure nil
  :commands maxima)

During initialization of Emacs, Amx gets initialized right after

(use-package amx
  :config
  (amx-mode 1))

is run. It tries to build the variable amx-cache against Emacs' global variable obarray, which doesn't include the command maxima because it is not yet interned. The variable amx-cache later becomes the absolute reference for anything amx does, including updating amx-history and amx-data, and saving them to local file e.g. ~\.emacs.d\amx-item. Writing to local file only happens upon kill-emacs, which explains why some commands are gone after restarting Emacs.

So I think the key takeaway here is to load Amx as late as possible so that all commands you use are already interned. If you use use-package, the easiest thing to do is to put :defer t:

(use-package amx
  :defer t
  :config
  (amx-mode 1))

Otherwise, just put Amx related commands (e.g. (amx-mode 1)) near the end of Emacs init file.

I don't know if anything can be done on Amx's end to improve the internal logic and get more robust loading. But it would be nice to mention that late loading of Amx is desirable in README.

DarwinAwardWinner commented 2 years ago

Amx is supposed to detect when new commands are defined, so this sounds like a possible bug. I will try to investigate when I can.

DarwinAwardWinner commented 2 years ago

Ok, so this is a bug that amx has inherited from smex. I think I understand why it happens. The function that places the history items at the front of the list is amx-restore-history, which is called at the end of amx-rebuild-cache. However, history items can only be placed at the front of the list if they actually exist in the list, so as you've surmised, when amx-initialize runs during initialization before a particular command has been defined, then the command doesn't make it into the list in time for amx-restore-history to promote it. It will get added to the list later when the command is defined, but now it's too late for it to get promoted.

I think I have a fix for this. Can you please install the bleeding-edge branch and tell me if it resolves your issue?

ywwry66 commented 2 years ago

Thanks for the quick fix! At first I was confused how this can be the solution by only removing the commandp test for the command names in amx-item file, since I originally suspected the names are not yet interned by the time amx-rebuild-cache is called. But it turns out I was wrong because all the command names in amx-item file are already interned by function amx-load-save-file, which is called just before amx-rebuild-cache in amx-initialize. Very elegant solution!

ywwry66 commented 1 year ago

I believe the fix should be good to merge to the main branch. Thanks!