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

Improve :pre-body hook semantics or add new hook #359

Open
madand opened this issue Jan 24, 2020 · 11 comments · May be fixed by #368
Open

Improve :pre-body hook semantics or add new hook #359

madand opened this issue Jan 24, 2020 · 11 comments · May be fixed by #368

Comments

@madand
Copy link

madand commented Jan 24, 2020

The code from :pre-body hook is called only when a hydra is activated by hydra-something/body command, but it is ignored when the hydra is activated implicitly by directly calling one of its heads (e.g. hydra-something/my-command).

In Spacemacs we have a thin abstraction on top of Hydra called a Transient State. And it is very common to activate transient state by directly calling one of its sub-commands (hydra heads), thus bypassing a call to hydra-something/body. I discovered this peculiarity of :pre-body while investigating a Spacemacs issue syl20bnr/spacemacs#13176.

I see 3 possible solutions for the above issue:

  1. Adjust :pre-body hook semantics, so it is also called when a hydra is activated by direct call to its head.
  2. Add new hook (e.g. :pre-hydra) with the described semantics and let :pre-body be as before.
  3. Reject this issue on hydra level. Then, I will hack the Spacemacs-specific solution on top of the existing :pre and :pre-body hooks.

If you choose 1 or 2, I am ready to make a PR.

Looking forward for your feedback @abo-abo.

@madand
Copy link
Author

madand commented Feb 29, 2020

@abo-abo ping

@abo-abo
Copy link
Owner

abo-abo commented Feb 29, 2020

Please PR with the first approach.

@madand
Copy link
Author

madand commented Feb 29, 2020

OK, thanks for prompt response.

madand added a commit to madand/hydra that referenced this issue Mar 10, 2020
Arrange for :body-pre callback to be called in case a hydra gets activated by
directly calling its head (e.g. `my-hydra/my-command`).

fixes abo-abo#359
madand added a commit to madand/hydra that referenced this issue Mar 10, 2020
Arrange for :body-pre callback to be called in case a hydra gets activated by
directly calling its head (e.g. `my-hydra/my-command`).

fixes abo-abo#359
madand added a commit to madand/hydra that referenced this issue Mar 10, 2020
Arrange for :body-pre callback to be called in case a hydra gets activated by
directly calling its head (e.g. `my-hydra/my-command`).

fixes abo-abo#359
@madand madand linked a pull request Mar 10, 2020 that will close this issue
@abo-abo
Copy link
Owner

abo-abo commented Mar 11, 2020

Thanks. I need an Emacs Copyright assignment for changes over 15 lines. Are you willing to get it?
See CONTRIBUTING.org for more info.

@madand
Copy link
Author

madand commented Mar 13, 2020

Yes, I am willing to get it. Already sent an email to GNU. Will keep you posted here.

@madand
Copy link
Author

madand commented Jun 16, 2020

Assignment paperwork is done, here is the final signed document: https://pub.madand.net/gnu/Kmit.1505691.GNU.EMACS.pdf

Please let me know if anything more needs to be done for this PR to be merged.

@madand
Copy link
Author

madand commented Jul 8, 2020

@abo-abo ping. See previous comment regarding the copyright assignment and #368 with the PR itself.

@madand
Copy link
Author

madand commented Sep 5, 2020

@abo-abo ping

@madand
Copy link
Author

madand commented Feb 8, 2021

Hi there, @abo-abo.
GitHub has notified me that the Spacemacs' syl20bnr/spacemacs#13176 (comment) was marked stale. Since "the right way" fix for that issue depends on #368 being merged into Hydra, I wonder whether you are still willing to merge it?

@madand
Copy link
Author

madand commented Sep 18, 2021

@abo-abo ping

@lebensterben
Copy link

@abo-abo
Hi I'm a co-maintainer of Spacemacs, please let us know whether you're still interested in the said PR.

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 a pull request may close this issue.

3 participants