Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix paging with God mode #247

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

darth10
Copy link

@darth10 darth10 commented May 29, 2020

Paging in which-key is not working when C-h(help-char) is entered in God mode.
This bug has been reported recently (emacsorphanage/god-mode#114), but it's been an issue for a while.

Current behaviour:
which-key god-mode orig-issue

This PR attempts to fix this issue.
The changes can be summarised as follows:

  • Use nadvice.el instead of advice.el.
  • Fix paging with God mode using an :around advice for god-mode-lookup-key-sequence.

Behaviour with these changes:
which-key god-mode fixed

Known issues:

  • There's currently an issue when which-key-undo-key is invoked from the help menu. From what I can tell, this is caused by which-key--update or which-key--hide-popup being invoked through a timer. I've tried calling which-key--stop-timer in the new advice function appropriately, but then the paging of which-key-undo-key has the same issue. Also, I'm not sure this is the best way to fix the problem. Here's what the behaviour looks like with the possible fix using which-key--stop-timer (ebcc4e6):

    which-key god-mode issue 1

    It would be great if I could get some clues on how to fix this issue.
    However, if it's still complicated, this issue could be tracked separately and fixed later on.

Any feedback is more than welcome!

@darth10
Copy link
Author

darth10 commented Jun 8, 2020

@justbur It would be really helpful if you could provide any feedback on this PR.

@justbur
Copy link
Owner

justbur commented Jun 13, 2020

I'm not very familiar with god-mode to start with, but after some basic testing it looks like god-mode breaks the basic emacs mechanism of using help-char to get help in the middle of an incomplete key sequence. In vanilla emacs, after say C-x emacs waits for you to complete the key sequence. If in an incomplete sequence you type help-char, which is C-h by default you get the command stored in prefix-help-command, which is describe-prefix-bindings by default. which-key takes over this functionality to do paging.

I can't trigger describe-prefix-bindings in god-mode in vanilla emacs (meaning with nothing else enabled) which points to a basic problem with how god-mode works. Neither x h nor x C-h works.

It seems to me that fixing this aspect of god-mode without worrying about which-key should be the first approach. If that is impossible for some reason, we can explore using the advice mechanism in which-key.

@darth10 darth10 marked this pull request as draft June 13, 2020 21:41
@darth10
Copy link
Author

darth10 commented Jun 13, 2020

I'll try modifying God mode to call prefix-help-command on entering help-char and follow up soon.
I've converted this PR to a draft for now.

@justbur
Copy link
Owner

justbur commented Jun 13, 2020

Great, thanks for your work.

@darth10
Copy link
Author

darth10 commented Jun 15, 2020

@justbur I've managed to fix God mode's handling of help-char.
This was done in commit emacsorphanage/god-mode@e80b6e6 as part of emacsorphanage/god-mode#115.

With the way God mode uses the command loop, this was possible only with the use of unread-command-events to add key sequences to the command loop.
Even with this change, help-char somehow fails to call which-key-C-h-dispatch when which-key-mode is enabled.
I'm open to suggestions for making the implementation in God mode better in any way.

That being said, as emacsorphanage/god-mode@e80b6e6 adds the god-mode-help-char-dispatch function to handle help-char, adding advice functions to call which-key-C-h-dispatch for paging becomes a bit easier.
I've updated this PR to do just that.

However, the timer issue with which-key-undo-key I described earlier is still present.
Any feedback to help fix this issue is more than welcome.
Alternatively, this particular issue can also be tracked and fixed separately.

@darth10 darth10 marked this pull request as ready for review June 15, 2020 09:41
@darth10
Copy link
Author

darth10 commented Jul 11, 2020

@justbur Have you had a chance to review my fix in God mode and the changes in this PR?

@justbur
Copy link
Owner

justbur commented Jul 16, 2020

I asked a couple of questions about the changes a while ago and was waiting to hear back

@darth10
Copy link
Author

darth10 commented Jul 16, 2020

Please refer to this comment.

I've managed to fix God mode's handling of help-char.
This was done in commit emacsorphanage/god-mode@e80b6e6 as part of emacsorphanage/god-mode#115.

With the way God mode uses the command loop, this was possible only with the use of unread-command-events to add key sequences to the command loop.
Even with this change, help-char somehow fails to call which-key-C-h-dispatch when which-key-mode is enabled.
I'm open to suggestions for making the implementation in God mode better in any way.

So, help-char now calls prefix-help-command in God mode, but the issue with which-key is not resolved.
I've updated this PR with simpler advice functions, which was possible due to the help-char fix in God mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants