emacs-helm / helm

Emacs incremental completion and selection narrowing framework
https://emacs-helm.github.io/helm/
GNU General Public License v3.0
3.37k stars 390 forks source link

[Display not Ready] when using keyboard macros #982

Closed tnielens closed 9 years ago

tnielens commented 9 years ago

When creating a keyboard macro (simple one, just call "helm-M-x keep li" (keep-lines)) I get the same problem as the one from #556. The default value of helm-exit-idle-delay was set to 0. Creating the macro and executing it in step-edit works fine but a simple execution of it fails. 0 seems to be too short for the completion list to have even one entry.

thierryvolpiatto commented 9 years ago

Don't use helm commands in macros, it is not supported.

tnielens commented 9 years ago

So you are supposed to disable the helm-mode anytime you want to make a macro involving completion ?

thierryvolpiatto commented 9 years ago

tnielens notifications@github.com writes:

So you are supposed to disable the helm-mode anytime you want to make a macro involving completion ?

helm-mode have nothing to do with helm-M-x. What you are probaly using is execute-extended-command helmized by helm-mode and not helm-M-x. If you really need M-x without helm, bind execute-extended-command in helm-completing-read-handlers-alist. Generally kmacros are used with keybindings, people use rarely M-x in their macros. here a simple example to add ";;" comments at each beginning of lines:

1) Hit f3 to start recording kmacro. 2) Hit ;; C-n C-a 3) Hit f4 to record kmacro 4) M-x helm-execute-kmacro 5) Hit C-j repetitively

Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

tnielens commented 9 years ago

Putting nil to the associated value of execute-extended-command in helm-completing-read-handlers-alist works fine, thanks. But still as you pointed out helm-M-x, and execute-extended-command with helm-mode both don't support to be executed as part of a keyboard macro by default. I still use it from time to time that command to execute unbound commands in macros and it'd be great to support it. Maybe add a more explicit message to the user saying that kmacros are not supported and that he should call commands without helm completion.

thierryvolpiatto commented 9 years ago

The error with [display not ready] have been fixed, but for some reasons the command created from helm-M-x will not work properly anyway when called from inside the macro, so now when defining macros, helm will return an error to prevent using it in macros.

michael-heerdegen commented 9 years ago

I think it would be more user friendly to fall back to vanilla execute-extended-command instead of raising an error.

Does this kind of bug occur with helminized commands, too? I.e., is it allowed to use completing-read while defining a keyboard macro and helm-mode on? If not, I think we could fall back to vanilla completion in this case, too.

thierryvolpiatto commented 9 years ago

Michael Heerdegen notifications@github.com writes:

I think it would be more user friendly to fall back to vanilla execute-extended-command instead of raising an error.

Sure, this is one option, but the best would be to handle kmacros, but I don't know if it is possible as kmacro is built with emacs vanilla behavior in mind.

I guess kmacro should handle such cases instead of assuming M-x is execute-extended-command and vice-versa. execute-extended-command accept two args and this is used nowhere in emacs. When kmacro record commands it should record in the M-x case the prefix arg and the command executed by M-x and run execute-extended-command with these two args, ignoring all the events between M-x and RET.

Does this kind of bug occur with helminized commands, too?

Of course. Note that the bug with [display not ready] is now fixed, but the macro recorded with helm commands, even if it doesn't throw error will not work as expected in most cases.

I.e., is it allowed to use completing-read while defining a keyboard macro and helm-mode on? If not, I think we could fall back to vanilla completion in this case, too.

Maybe, but for now we just raise an error as my dedicated time to fix this as expired, maybe later...

Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

michael-heerdegen commented 9 years ago

Thierry Volpiatto notifications@github.com writes:

Maybe, but for now we just raise an error as my dedicated time to fix this as expired, maybe later...

Maybe I can have a look in the meantime. Do you have a guess what causes the macros not to work correctly? What did you try and how did the recorded macros fail when executed?

thierryvolpiatto commented 9 years ago

Michael Heerdegen notifications@github.com writes:

Maybe I can have a look in the meantime.

Thanks.

Do you have a guess what causes the macros not to work correctly?

The problem is kmacro is expecting a minibuffer completion, I mean that at end of completion with regular M-x the command is in the minibuffer just before pressing RET. With helm the contents of minibuffer doesn't change, what is executed when you press RET is what is in helm-selection, not minibuf. To understand better, you can use `kmacro-edit-macro-repeat' (C-x C-k C-e).

Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto commented 9 years ago

I have finally fixed by falling back to a regular emacs completion when defining or executing kmacros as it seems to me that the way kmacros are implemented is really incompatible with helm.

Of course if one use another command with helm completion she will encounter same problem.

Maybe I should disable helm-mode at upper level to handle this, but we will still have anyway the same problem with native helm commands... Will see.

Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

michael-heerdegen commented 9 years ago

Thierry Volpiatto notifications@github.com writes:

I have finally fixed by falling back to a regular emacs completion when defining or executing kmacros as it seems to me that the way kmacros are implemented is really incompatible with helm.

I came to the same conclusion.

Executing a keyboard macro is not much different from just pushing the events to "unread-command-events".

The problem with Helm is that it's completion mechanism is based on timers. Completion is performed in a timer.

When executing a keyboard macro, Emacs doesn't ever get idle. So, no timer fires in the meantime.

If we would want to make keyboard macros work with Helm, we would have to reimplement the basic functions without timers, emulating a user interaction.

I don't think that's worth it. Falling back to vanilla completion is totally legitimate here, this may even be a good idea anyhow when this bug did not exist.