Make (god-mode) always work #143
Conversation
It's often been the case that I'll hit the `god-mode` key and it does nothing, because `god-global-mode` and `god-local-mode` is out of sync. Either I wanna turn on god-mode, because it's off locally, but it's on globally (and there's no indication to show that), so the key (seemingly) does nothing. It only affects background buffers (and sometimes in ways I don't even want). Or, I wanna turn off god-mode, but unbeknownst to me it's already off globally, it's just off locally. So again the key does nothing in the active buffer, only weirds up the background buffers. With this patch, if `god-global-mode` and `god-local-mode` has gotten out of sync, this key syncs them up by changing the local-buffer which is always what I want. If they are already in sync, it toggles them (i.e. the old behavior, which is exactly what you want when they are in sync).
|
OK, I'll redo this pull request normally in a separate branch. |
|
What's If you're worried about |
Have you looked at Visual indicators for God mode in the README? |
It's a variable that
That's not the issue. I'll try to explain again down below.
Yes, I agree with that.
That's not what this patch does. The patch is not about always keeping global in sync with local. If that was the take-away, I was being unclear in my initial explanation about what the problem is.
Yes, cursor shape, color, and mode-line indicator are all great to see whether So without the patch (I'll refer to running
It's been something that has been going on for years and I've been like "oh, god-mode just sometimes doesn't work, or maybe it's that I'm inputting the key command incorrectly" and now when I read the source code I realized why the key had been so unreliable all this time and often needed to be hit twice. (Which, while typing quickly, even though there are visual indicators, is not good; we don't pause to look after every key press.) With the patch:
When we run the god-mode function when the global state is counter to the local state, it's because we don't realize that the global state is counter to the local state, because what we're the most concerned with is the local state. Put differently, the intended semantics when hitting the key when god-mode is off locally is that we want to turn it on both globally and locally, and when hitting the key when god-mode is on locally is that we want to turn it off both globally and locally. That's what this patch does. The current semantics, without this patch, which I believe are not good, is that the key ignores the local state (even though the local state is the only state the user is aware of [through cursors, colors, modeline indicators, maybe even other theme changes]), toggles the global state (even though the user has no indication of the global state), and only then sets the local state to match the global state (even if they were already matching so hitting the key did nothing locally). |
|
Thanks for the detailed explanation, @snan! I get the use case now, but I want to understand more.
If I try these steps with
Let's assume you have bound |
|
Interesting! Yeah, I have one I thought most people had god-mode set to I've actually never used god-mode-all 💁🏻♀️ |
|
Would it be possible for you to try using Your change makes sense, but this is starting to sound more like a documentation (docstring and README) inconsistency to me. |
|
The problem with god-mode-all is that it toggles and changes all the background buffers too ( It's not only a documentation issue; god-mode often seemingly does nothing making it kind of useless. One alternative is to name the god-mode I submitted something new and then keep the old god-mode but… that makes the old god-mode useless and confusing for new people who might, like me, go for years just not really trusting the key and thinking that "sometimes it just doesn't work". |
|
Your change doesn't fix this case:
I did the following steps with your change:
I might have set some other state while I was testing calls to I'll try to dig deeper soon, but the intended behaviour of the The right thing to do here would be to remove the |
|
The idea that I want is: If I am in a buffer with god-mode locally off, make sure god-local-mode and god-global-mode is on, and without mapc' ing every buffer because I already have god-mode-all for that, and I also have god-local-mode for when I only want to change the local god-mode.
If that's really with my version of god-mode (could you double check that? because that sounds like the old behavior), then I've made a bug. The code I wrote compares god-global-mode and god-local-mode. |
|
Ah you'e right! Investigating it now |
|
Found it! Last line needs to be |
|
I'll clean that up and test it out soon, but I think the function should end up looking something like this: (defun god-mode ()
(interactive)
(god-local-mode (if (bound-and-true-p god-local-mode) -1 1))) |
That's not correct, |
|
@snan Just an update... I've committed the changes you sent via email on the |
|
After a brief look (and sorry if I'm going back in circles here), I really think this is a configuration/documentation issue.
If you configure Emacs with the above guidelines, The I'll update the README with this information. |
|
In that case, I'll have to make a fourth function with a new name to do what my version of (god-mode) does. Because god-mode-all isn't what I want, I don't wanna mapc over every single other buffer in case they're already set up perfectly. Let's say I have god mode on in most buffers but I turned it off on one (probably in an rcirc bot query since god-mode can sometimes be good, sometimes bad in those) and then I'm in a text file and god-mode-local gets turned off. I don't wanna call god-mode-all in that situation since it'd also turn god-mode on in those backgrounded buffers where I have previously turned it off. |
That's a good approach if that works for you.
This is the use case that I don't entirely understand. The condition "God mode is on in most buffers" is not 100% clear, as it doesn't describe the state of |
|
I'll try to explain it in two ways. \1. The use-case: It's more of a super version of local (that also changes future buffers) than a weak version of -all. \2. The semantics: The original behavior for If local & global is the same, toggle both. Otherwise, do nothing in the local buffer but toggle global (secretly, invisibly, in the background) so they match. As user, not even sure something happened or if I even hit the key correctly. But with my style: But with the original, every hit of the key alternates between on, off, on, off and makes sure global and local is synced up except that first hit of the key, which only syncs up with no immediately noticable effect. |
That makes sense.
That's right, but the Thanks for the details though! |
2 participants
It's often been the case that I'll hit the
god-modekey and it does nothing, becausegod-global-modeandgod-local-modeis out of sync.Either I wanna turn on god-mode, because it's off locally, but it's on globally (and there's no indication to show that), so the key (seemingly) does nothing. It only affects background buffers (and sometimes in ways I don't even want).
Or, I wanna turn off god-mode, but unbeknownst to me it's already off globally, it's just off locally. So again the key does nothing in the active buffer, only weirds up the background buffers.
With this patch, if
god-global-modeandgod-local-modehas gotten out of sync, this key syncs them up by changing the local-buffer which is always what I want. If they are already in sync, it toggles them (i.e. the old behavior, which is exactly what you want when they are in sync).=================================================================
Use a dedicated feature branch
Please use a dedicated feature branch for your feature request, instead of asking us to merge "your-fork/master" into the "origin/master". The use of dedicated branches makes it much more convenient to deal with pull-requests, especially when using Magit to do so.
If you were about to open a pull-request asking us to merge your version of "master", then see 1 for instructions on how to quickly fix that and some information on why we ask you to do so.
Additionally we ask you to allow us to push to the branch you want us to merge. We might want to push additional commits and/or make minor changes. Please make sure the box next to "Allow edits from maintainers" is checked before creating the pull-request.
=================================================================
Do NOT use Github to edit files and create commit messages
Unless you are aware of all the pitfalls and take great care to avoid them, the use of Github results in many small defects, including but not limited to trailing whitespace, commit messages containing overlong lines and no newline at the very end, and "GitHub [email protected]" being used as the committer. The last one cannot even be avoided, which I consider as an affront.
Github is an insufficient interface for editing files and creating commits. Please don't do it when contributing to this project.
=================================================================
What you should write here
Please summarize the changes made in the commits. Explain why you are making these changes, not just what changes you are making. This also applies to the commit messages.
=================================================================
How to update the manual
If this project has a manual and you make changes to that, then edit only "PROJECT.org". Do not manually edit "PROJECT.texi". The latter has to be generated from the former. If you want to do that yourself, then follow the instructions at 2. Otherwise a maintainer will do it and amend that to your commit.