Emacsclient fix for missing face background

[GarySGlover]

Issues starting new frame with emacsclient when using deamon mode from system service. Traced down tbe following call in indent-bars--main-color no being able to get a color returned.

(funcall (if face-bg #'face-background #'face-foreground) main))

Fix is to only call the indent-bars-setup on a new frame if it is definitely a frame on a graphical display

(when (display-graphic-p frame)
    (indent-bars-setup)))

Other changes are ensure the indent-bars-setup is called on a new frame if the displayed buffer has the indent-bars-mode enabled.

[jdtsmith]

Thanks for this contribution. Is this because the daemon allocates a non-GUI hidden frame? Will add some comments in the code.

[jdtsmith]

Overall I'm wondering if this much simpler fix could achieve the same thing:

modified   indent-bars.el
@@ -1248,8 +1248,9 @@ Adapted from `highlight-indentation-mode'."

 (defun indent-bars-setup-and-remove ()
   "Setup indent bars and remove from `after-make-frame-functions'."
-  (remove-hook 'after-make-frame-functions #'indent-bars-setup-and-remove)
-  (indent-bars-setup))
+  (when (display-graphic-p)
+    (remove-hook 'after-make-frame-functions #'indent-bars-setup-and-remove)
+    (indent-bars-setup)))

 ;;;###autoload
 (define-minor-mode indent-bars-mode

[GarySGlover]

Thanks for this contribution. Is this because the daemon allocates a non-GUI hidden frame? Will add some comments in the code.

Not quite sure how Emacs is working this part behind the scene. But when I started a new frame, the after-make-frame-functions seem to be getting called twice, but the first one isn't fully graphical with the colors set.

Overall I'm wondering if this much simpler fix could achieve the same thing:

modified   indent-bars.el
@@ -1248,8 +1248,9 @@ Adapted from `highlight-indentation-mode'."

 (defun indent-bars-setup-and-remove ()
   "Setup indent bars and remove from `after-make-frame-functions'."
-  (remove-hook 'after-make-frame-functions #'indent-bars-setup-and-remove)
-  (indent-bars-setup))
+  (when (display-graphic-p)
+    (remove-hook 'after-make-frame-functions #'indent-bars-setup-and-remove)
+    (indent-bars-setup)))

 ;;;###autoload
 (define-minor-mode indent-bars-mode

The current function added in the minor mode startup is a lambda, so doesn't get removed by the remove-hook. I did get fix together closer to this one, but if I removed the function whilst the mode was still active, closing the last graphical frame and then opening a new one wouldn't show the bars. This is the cleanest I've got that seems to always give the correct display for this workflow

• Start in daemon mode
• Open new frame with indent-bars-mode enabled => indent-bars shown
• Close last opened frame and open new frame => indent-bars shown
• Disable indent-bars-mode
• Close last opened frame and open new frame => indent-bars not shown
• Re-enable indent-bars => indent-bars shown
• Close last opened frame and open new frame => indent-bars shown

Removing the hook would end up with indent-bars not showing if I closed the last frame and opened a new one. Not removing the hook in the teardown would end up with indent-bars showing in a buffer that was previously enabled but now disabled when closing and re-opening the last frame.

I've also tested in non daemon mode, with single and multiple frames, and doesn't seem to have any negative impact that I can tell.

[jdtsmith]

The current function added in the minor mode startup is a lambda, so doesn't get removed by the remove-hook

Oh right, that's dumb of me; not removing the hooks is clearly a problem (and may explain some other misfeatures I've seen even without a daemon running). This is what I get for making "blind" fixes (sadly I don't have an emacs which supports --daemon).

So maybe a simple:

modified   indent-bars.el
@@ -1246,10 +1246,11 @@ Adapted from `highlight-indentation-mode'."
   (indent-bars-teardown)
   (indent-bars-setup))

-(defun indent-bars-setup-and-remove ()
-  "Setup indent bars and remove from `after-make-frame-functions'."
-  (remove-hook 'after-make-frame-functions #'indent-bars-setup-and-remove)
-  (indent-bars-setup))
+(defun indent-bars-setup-and-remove (frame)
+  "Setup indent bars for FRAME and remove from `after-make-frame-functions'."
+  (when (display-graphic-p frame)
+    (remove-hook 'after-make-frame-functions #'indent-bars-setup-and-remove)
+    (indent-bars-setup)))

 ;;;###autoload
 (define-minor-mode indent-bars-mode
@@ -1258,11 +1259,8 @@ Adapted from `highlight-indentation-mode'."
   :group 'indent-bars
   (if indent-bars-mode
       (if (and (daemonp) (not (frame-parameter nil 'client)))
-     (let ((buf (current-buffer)))
-       (add-hook 'after-make-frame-functions
-             (lambda () (with-current-buffer buf
-                  (indent-bars-setup-and-remove)))
-             nil t))
+     (add-hook 'after-make-frame-functions
+           #'indent-bars-setup-and-remove nil t)
    (indent-bars-setup))
     (indent-bars-teardown)))

Removing the hook would end up with indent-bars not showing if I closed the last frame and opened a new one. Not removing the hook in the teardown would end up with indent-bars showing in a buffer that was previously enabled but now disabled when closing and re-opening the last frame.

indent-bars operates buffer by buffer. The frame should have very little do with anything, other than that it controls whether faces are defined yet if buffer is initialized inside a "fake frame". I hope the above in which the setup is actually removed works; can you give it a test?

[jdtsmith]

I pushed this diff up to master since mine was so clearly wrong of a hook; give it a test and let me know how it goes?

[GarySGlover]

Get nil for the background color with that version. Needs the (select-frame frame) I can get rid of the (when (display-graphic-p frame) without change of your version and get the same result. So it seems like the after-frame function isn't running on the right frame (maybe against the daemon frame) by default.

With that fix in place though I have an issue with trying to disable the mode. If I disable the mode then open a frame, the mode variable stays disabled but the bars start apeearing again. The hook removal needed to be (remove-hook 'after-make-frame-functions 'indent-bars-setup-and-remove t) to remove the hook correctly.

Fixing that leads to the next issue, which is now with the hook removed, if I close the last frame with the mode enabled and open a new frame with emacsclient I don't get the bars. The fix I have for that is to change to the indent-bars-frame-setup function and move the hook removal to the teardown.

Having done that, I end up with my last issue. That is if I disable the mode the hook gets removed. Enabling the mode again works up until closing the last frame. When opening a new frame at this point the bars are not showing even thought the mode is enabled. Which is the final fix to refactor the minor-mode function to recreate the hook every time.

[jdtsmith]

Sigh, sorry, I fixed the hook removal so it should at least work.

But again, frames simply should not matter here (modulo "fake frames" that don't have face information available yet and trip up the setup, which honestly is kind of crummy Emacs behavior if you ask me).

Either indent-bars-mode is enabled in a buffer, or it isn't. Setup simply sets many buffer-local variables related to font-lock. For example, if you examine the variable font-lock-keywords in a buffer, you should see indent-bars-ts--display on the list (probably near the end).

So we need to determine what it is about your various frame operations that is messing up the operation of setup. If you can proceed step by step and provide the following info, that will help:

1. How did you create a frame? Is it visiting a new buffer? Was this via emacsclient or normal frame operations?
2. Is indent-bars-mode = t in the buffer?
3. Is indent-bars-ts--display on font-lock-keywords?
4. What is the value of indent-bars--main-color?
5. If something is wrong with the above, does M-: (indent-bars-setup) fix it?

Check these for various combinations of ways to make frames and hopefully we can figure it out.

[GarySGlover]

Had to put in the (select-frame frame) to be able to open first frame. But I'll answer the rest based on having that and for 2-5 I'm going to answer based on having opened/closed/reopened the frame, which is when I'm not getting the bars correctly now.

1. Emacs daemon is running as a Linux service. Opening frames using emacsclient -c into the existing scratch buffer. This has the mode already enabled through the prog-mode hook.
2. Yes
3. Can see indent-bars--display in the font-lock-keywords
4. #a7a7cbfedd0f
5. Yes that brings the bars back

[jdtsmith]

I don't know what you mean about select-frame. What happens without it?

Also, how do you see bars in an empty scratch buffer?

Instead of 5, what if you do M-x font-lock-fontify-buffer?

What happens for all of these if you make a frame the old fashioned way (with C-x 5 2 for example)?

[GarySGlover]

Without the (select-frame frame) from my version, the background color is nil. That causes the window to close. That's need rather than (when (display-graphic-p frame) in the indent-bars-setup-and-remove function.

When I open first time, I'm adding a few spaces and some garbage just to allow the bars to show. Sorry should have added that bit of info.

C-x 5 2 creates a new frame and does resolve.

font-lock-fontify-buffer doesn't help

Running the (indent-bars--create-faces 9 'reset) line from the setup seems to be enough to get the bars back.

[jdtsmith]

One frame will be a console frame, which is always running for the daemon, so I don't think we should do anything there.

Running the (indent-bars--create-faces 9 'reset) line from the setup seems to be enough to get the bars back.

Interesting. So the faces are not defined; can you M-x describe-face indent-bars-1 prior to attempting the fix (with create-faces)?

Is this always with the same daemon/server running or are you restarting it?

[GarySGlover]

indent-bars-1 is undefined for all values before the reset. After the reset foreground and inherit are set.

I'm keeping the same daemon running during the tests while closing/opening frame.

[GarySGlover]

With enough spaces to test, indent-bars-10 and above stay working it's just 1-9 that lose their face values

[GarySGlover]

Ii I remove (indent-bars--create-faces 9 'reset) from the setup, it all seems to work as expected.

[jdtsmith]

OK, interesting. Does it also work if you just remove the'reset? You can see that with 'reset there, the face is reset then immediately redefined, so it's not clear why 'reset would do something untoward.

(if (and redefine (facep face)) (face-spec-reset-face face))
(face-spec-set face (indent-bars--calculate-face-spec i))

Here's a test to try: while the frame is active, before you have "fixed" the faces, M-: (face-spec-set 'indent-bars-1 (indent-bars--calculate-face-spec 1)). Does that fix it (for one bar)?

[GarySGlover]

Removing 'reset fixes it.

(face-spec-set 'indent-bars-1 (indent-bars--calculate-face-spec 1)) does bring back the first bar, but disappears again after closing last frame.

[jdtsmith]

Can you clarify "after closing the frame"? Is your operation to open one frame with emacsclient, do some stuff, then close it (having no GUI frames?). That could be the issue: perhaps all face definitions are lost when the final GUI frame closes. I still don't know why removing 'reset should matter, but I think it does no harm; can you test out the latest (main)?

[GarySGlover]

Yeah, process is daemon running in background. Open emacsclient, do some suff, then close the single GUI frame, with the daemon still running in the background.

Have tried that code, still need the below update to the setup-and-remove. Other than that is now working as expected, bars showing when enabled in all circumstances and not showing once disabled.

(defun indent-bars-setup-and-remove (frame)
  "Setup indent bars for FRAME and remove from `after-make-frame-functions'."
  (select-frame frame)
  (remove-hook 'after-make-frame-functions #'indent-bars-setup-and-remove t)
  (indent-bars-setup))

[jdtsmith]

OK, I used a slightly different approach that should also work. Please test.

[GarySGlover]

That words perfectly. I'll close this PR. Thanks for your help on this.

[jdtsmith]

Great. Thanks for your patience trying to understand this.