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

Make hooks to set up extensions removable #261

Open
josephmturner opened this issue Oct 22, 2024 · 1 comment
Open

Make hooks to set up extensions removable #261

josephmturner opened this issue Oct 22, 2024 · 1 comment

Comments

@josephmturner
Copy link
Contributor

josephmturner commented Oct 22, 2024

Hi @nobiot!

@riscy suggested in this comment that hyperdrive-org-transclusion add hooks in a mode or setup function so that merely requiring the library doesn't irrevocably affect the Emacs runtime. After reading Chris's comment, I moved the add-hook form into a minor mode which can be disabled to remove the hook.

Would you be open to making a change like this in org-transclusion so that the built-in extensions can be enabled/disabled with autoloaded minor modes?

The downside I see is that the setup instructions would have to change. For example, setting up org-transclusion-indent-mode extension would require users to change

(with-eval-after-load 'org-transclusion
     (add-to-list 'org-transclusion-extensions 'org-transclusion-indent-mode)
     (require 'org-transclusion-indent-mode))

to

(with-eval-after-load 'org-transclusion
    (org-transclusion-indent-mode +1))

I'm willing to draft a patchset. Are you open to such a breaking change?

Thank you!

Joseph

@nobiot
Copy link
Owner

nobiot commented Oct 25, 2024

@josephmturner , Thank you for bringing this up. I agree with @riscy on "simply bringing your package in as a dependency shouldn't modify the Emacs runtime with an un-undoable add-hook." And I would have thought I was doing this -- it sounds like my lack of knowledge betrayed my intention.

Let me review my code in more detail and come back to you. In general, I am open to "breaking change"; but we may need a period where we support the "old way" in parallel, issue a warning, and keep it for 6-12 months.

nobiot added a commit that referenced this issue Jan 2, 2025
Added the  following utility functions to support the move

- `org-transclusion-extension-functions-add-or-remove`
- `org-transclusion-extension-set-a-hook-functions`

Modified function `org-transclusion-load-extensions-maybe` so that setting
`org-transclusion-extensions` with the `customize` facilities will automatically
activate the corresponding minor mode for each extension.

Use of `customize` and `org-transclusion-extensions` is optional. Users can
ignore these and use extensions by loading and activating minor modes in their
init.el file.

So, this below should work (each minor mode function has the autoload cookie, so
the library should not have to be explicitly loaded).

```
(with-eval-after-load 'org-transclusion
    (org-transclusion-indent-mode +1))
 ```

Adding to the list and requiring the library like this below did not use to be
necessary...

```
(with-eval-after-load 'org-transclusion
     (add-to-list 'org-transclusion-extensions 'org-transclusion-indent-mode)
     (require 'org-transclusion-indent-mode))
```

The Customizing facility would load the active extension library with
`require`.... But I think this was never intuitive.
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

No branches or pull requests

2 participants