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

Add global minor modes for features (take two) #46

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

progfolio
Copy link
Collaborator

  • exwm-core.el (exwm--define-global-minor-mode): Macro for arranging EXWM mode hooks, deprecating enabler functions, logging.
  • exwm-background.el: Add global mode definition, autoloads (exwm-background-enable): remove
  • exwm-randr.el (randr): as above (exwm-randr-enable): as above
  • exwm-systemtray.el (systemtray): as above (exwm-systemtray-mode): as above
  • exwm-xim.el (xim): as above (exwm-xim-enable): as above
  • exwm-xsettings.el (xsettings): as above (exwm-xsettings-enable): as above

@progfolio
Copy link
Collaborator Author

PROS:

  • uses macro to avoid implementation repetition.
  • defines deprecated "enable" function

CONS:

  • Requires autoload cookie dance (perhaps this could be fixed upstream for Emacs 31)
  • Hacky way of reproducing autoload cookie docstring in expansion (not entirely necessary, and could be made more robust)

@Stebalien
Copy link
Contributor

IMO, macroing the internals (as suggested by @minad in #45 (comment)) is probably going to be less painful. It may be a bit more code, but that extra code won't require knowledge of autoload cookies to understand.

@minad
Copy link
Member

minad commented Jun 4, 2024

@Stebalien Yes, let's better go with the simple solution. Hopefully in the future Emacs gets a more flexible mechanism for autoload generation.

@progfolio
Copy link
Collaborator Author

As you wish

@progfolio
Copy link
Collaborator Author

@Stebalien @minad I took the liberty of autoloading the enable functions, too. I see no reason not to.

@minad
Copy link
Member

minad commented Jun 4, 2024

I took the liberty of autoloading the enable functions, too. I see no reason not to.

I prefer if we don't do this. The enable functions are obsolete, haven't been autoloaded so far, so we should keep them not autoloaded, such that we don't encourage their use. Otherwise it looks good.

I will try your PR now.

@minad minad merged commit 5516f24 into emacs-exwm:master Jun 4, 2024
@minad
Copy link
Member

minad commented Jun 4, 2024

Thanks, merged. I've also updated the commentaries and the wiki, see https://github.com/emacs-exwm/exwm/wiki/Home/_compare/54a6528040108a4e9b804bd83c3bee0644eff903...039e3503e25c5c6736e77614216a90f7f1ba5b71.

@progfolio progfolio deleted the feat/global-minor-modes branch June 4, 2024 14:59
@minad
Copy link
Member

minad commented Jun 4, 2024

@progfolio Just to be sure if I recall correctly, you've assigned copyright to the FSF, right?

The next step will be to turn exwm-enable into a proper mode :)

@progfolio
Copy link
Collaborator Author

@progfolio Just to be sure if I recall correctly, you've assigned copyright to the FSF, right?

The next step will be to turn exwm-enable into a proper mode :)

I should have papers on file for contributions to Org-mode. I have contributed to Emacs as well, but I don't recall if I had to sign separate papers or the first signing covered everything.

@progfolio
Copy link
Collaborator Author

The next step will be to turn exwm-enable into a proper mode :)

Agreed here, too

@Stebalien
Copy link
Contributor

I should have papers on file for contributions to Org-mode. I have contributed to Emacs as well, but I don't recall if I had to sign separate papers or the first signing covered everything.

Should be the same thing. The org-mode license headers include:

;; This file is part of GNU Emacs.

@progfolio
Copy link
Collaborator Author

progfolio commented Jun 4, 2024

Should be the same thing. The org-mode license headers include:

We should be good in that case.
If need be, just let me know and I can sign again.

@medranocalvo
Copy link
Contributor

I wonder whether the minor modes should be named *-minor-mode. I had to do that when naming exwm-minor-mode because exwm-mode exists. Are there conventions?

@minad, @Stebalien: in general we should ask ELPA Maintainers to check copyright assignment for us. See https://github.com/emacs-exwm/exwm/wiki/Maintenance#check-copyright-assignment. Maybe it's not needed in this case, I defer to your judgement.

@progfolio: thank you and welcome to EXWM!

@minad
Copy link
Member

minad commented Jun 7, 2024

I wonder whether the minor modes should be named *-minor-mode. I had to do that when naming exwm-minor-mode because exwm-mode exists. Are there conventions?

We are probably going to call exwm-minor-mode differently, see the discussion in #53. I proposed names like exwm-window-manager-mode or exwm-wm-mode.

in general we should ask ELPA Maintainers to check copyright assignment for us.

Yes, I do that in case of doubt.

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.

4 participants