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

Introduce apheleia-inhibit-functions #138

Merged
merged 8 commits into from
Nov 12, 2022

Conversation

daanturo
Copy link
Contributor

@daanturo daanturo commented Oct 17, 2022

Fixes #134

(I accidentally force-pushed #135 from a shallow repo)

@daanturo daanturo force-pushed the conditionally-disable branch from 664b384 to 05ea832 Compare October 23, 2022 14:20
Copy link
Member

@raxod502 raxod502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this will be very helpful. I think we can improve the design a little bit, though. define-globalized-minor-mode already has a feature that is intended to accomplish conditional enabling of a minor mode:

Signature
(define-globalized-minor-mode GLOBAL-MODE MODE TURN-ON [KEY VALUE]... BODY...)

Documentation
Make a global mode GLOBAL-MODE corresponding to buffer-local minor MODE.

TURN-ON is a function that will be called with no args in every buffer
and that should try to turn MODE on if applicable for that buffer.

So, if we make the TURN-ON function check apheleia--not-inhibited before calling apheleia-mode, I think it will accomplish the same, but more simply.

I don't mind changing that in your PR directly but I wanted to ask you your thoughts first.

@daanturo
Copy link
Contributor Author

So, if we make the TURN-ON function check apheleia--not-inhibited before calling apheleia-mode, I think it will accomplish the same, but more simply.

I think that's totally fine. For me, my condition is a bit slow to run so I originally want to defer checking until saving is triggered, but I can just modify it to add to ".dir-locals-2.el" to evaluate just once anyway.

Cleaner code is superior, I'm in for define-global-minor-mode's TURN-ON.

@raxod502
Copy link
Member

Ok, cool. Yeah, I figured that if the user desired caching then they could simply implement that in their disable-function.

@raxod502
Copy link
Member

How does this version look to you?

@daanturo
Copy link
Contributor Author

daanturo commented Oct 28, 2022

We may need to dump those variables into the autoloads file, else apheleia-global-mode will:

Error caused by user's config or system: ~/.config/doom/config.el, (void-variable apheleia-inhibit)

Edit: I don't think apheleia-inhibit is really needed, we can just set apheleia-inhibit-functions to '(always) locally anyway. Just a bit more verbose config (that's already relatively advanced) can lighten the maintenance burden of a variable.

When running apheleia-mode-maybe without loading.
@daanturo daanturo force-pushed the conditionally-disable branch 3 times, most recently from 0999f44 to 66f30de Compare October 28, 2022 10:07
@daanturo
Copy link
Contributor Author

daanturo commented Nov 6, 2022

Ping @raxod502 , I added commit 66f30de .

@raxod502
Copy link
Member

Yeah, sorry for the delay, that seems fine to me. Would be equally okay to autoload the variable individually.

Looks like CI is failing, let me see if I can fix that.

@raxod502
Copy link
Member

Ah yes it's just checkdoc.

@raxod502 raxod502 enabled auto-merge (squash) November 12, 2022 03:28
@raxod502 raxod502 merged commit a82e40c into radian-software:main Nov 12, 2022
@raxod502
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

Conditionally disable apheleia-mode locally
2 participants