Skip to content

Conversation

@luispauloml
Copy link
Contributor

@luispauloml luispauloml commented Dec 24, 2022

I started using Emacs server and I want to make sure that god-mode is turned on globally every time a new frame is created. However, god-mode-all is only capable of toggling the mode, and I needed it to enforce the mode to be turned on, otherwise the following happens:

  1. Start server and god-mode is turned on from the init file
  2. Create a new frame which calls god-mode-all from after-make-frame-functions and it turns off.
  3. Turn it on manually and use the frame.
  4. Delete the frame when I'm done with god-mode still on.
  5. Create another frame which again calls god-mode-all from after-make-frame-functions and it turns off because it was left on.

If in (4) god-mode is off, then it will be turned on when I create a new frame, as I want it to be. But then I would need to remember to turn it off manually or put god-mode-all in delete-frame-functions, and that would only work if I deleted the frame while it was on. Now one can see the way it is right now god-mode-all is not the proper function for this task.

To achieve what I wanted, I added an optional argument to god-mode-all which controls whether the function toggles god-mode, or strictly activates or deactivates it. To ensure backwards compatibility, I made sure to make nil as the value for toggling the mode, and as for the positive and negative values for turning it on or off, I followed the convention of define-minor-mode.

In the new function, I could have bypassed mapc based on the god-global-mode variable, but this value is not consistent. What I mean is that god-mode could have various states for different buffers, and god-global-mode was not capable of reliably indicating whether all buffers had god-mode on or off, therefore the only way the new god-mode-all function would work properly is by not relying on this value.

As a side note, if all goes well, I will probably create other pull requests with improvements I had made for myself that I think others might also benefit from.

Add optional ARG to god-mode-all to make it also strictly activate or
strictly deactivate god-mode globally instead of just toggling it.
Making nil, i.e. no value passed as ARG, as the value for toggling
makes this change backwards compatible.
@darth10
Copy link
Collaborator

darth10 commented Dec 27, 2022

LGTM and thanks for the contribution @luispauloml!

I really appreciate the thinking around backward compatibility.
Any improvements are always more than welcome.

@darth10 darth10 merged commit 0c86471 into emacsorphanage:master Dec 27, 2022
@luispauloml luispauloml deleted the feat/god-mode-all-opt-arg branch December 28, 2022 13:44
@luispauloml
Copy link
Contributor Author

luispauloml commented Dec 28, 2022

Thank you, @darth10.

I'd like to take this chance and ask something here instead of creating an issue just for it.

I was planning to create another pull request to add customization for God mode's lighter, but now I want to start using god-mode-pause and I want to find a way for the same lighter to indicated whether the God mode is paused or not.

The problem is that the changes to show the paused stated will probably overwrite part of the changes I already made. Also, I think I will have to deal with the inconsistencies of god-global-mode I mentioned in the opening post in this request. For instance, I found a way to reach a state where the variables god-local-mode, god-global-mode and god-local-mode-paused are all true, which doesn't make sense and I'd like to fix it.

So I would like you to choose what you think is best for the history of the repository:

  1. I make a pull request just for the lighter and another one for the subsequent changes.
  2. I finish working on everything and make a bigger PR in which the original changes for the lighter will be spread across a few commits.

To make a decision, you can check the "feat/lighter-customization" branch in my fork for the lighter customization, which has only one commit (probably commit 23bb1e2 if I don't force push anything).

Honestly, I prefer option 1, because I don't even know if I'll be able to accomplish what I want, but I thought I might ask before moving forward just I case I do manage to do it.

@darth10
Copy link
Collaborator

darth10 commented Dec 28, 2022

The problem is that the changes to show the paused stated will probably overwrite part of the changes I already made. Also, I think I will have to deal with the inconsistencies of god-global-mode I mentioned in the opening post in this request. For instance, I found a way to reach a state where the variables god-local-mode, god-global-mode and god-local-mode-paused are all true, which doesn't make sense and I'd like to fix it.

This is an interesting problem, and I think it's worth exploring.
If it helps, take a look at #143 which talks about god-global-mode.

To make a decision, you can check the "feat/lighter-customization" branch in my fork for the lighter customization, which has only one commit (probably commit 23bb1e2 if I don't force push anything).

I think you meant luispauloml@bf20cbe 😄
And that looks like a great improvement.
Adding more customisation to God mode is always a good idea in general.

Honestly, I prefer option 1, because I don't even know if I'll be able to accomplish what I want, but I thought I might ask before moving forward just I case I do manage to do it.

I also prefer option 1 as I usually squash-and-merge commits.
IMO we should keep enhancements as separate as possible using commits.

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

Development

Successfully merging this pull request may close these issues.

2 participants