jdtsmith / indent-bars

Fast, configurable indentation guide-bars for Emacs
GNU General Public License v3.0
343 stars 14 forks source link

Does not work in emacsclient frame #10

Closed dinkonin closed 1 year ago

dinkonin commented 1 year ago

When I try the package on Emacs 29.1 on linux with this config:

(use-package indent-bars :elpaca (:repo "https://github.com/jdtsmith/indent-bars.git")
  :hook ((python-ts-mode yaml-ts-mode) . indent-bars-mode))

It works only when I start emacs with emacs pathto.yml, the bars are missing when I do emacsclient -n -c -a '' pathto.yml . Probably it detects that -a '' starts emacs in daemon-mode, if it's not running ?

jdtsmith commented 1 year ago

This works fine for me. How are you starting your emacs server, with (server-start) in your init, or --daemon? If the latter, can you let me know the values of the variables after-make-frame-functions and indent-bars-spacing when it fails to start?

dinkonin commented 1 year ago

Im starting my server like this

(use-package server :demand t
  :init
  (setq server-client-instructions nil)
  :config
  (unless (server-running-p)
    (server-start)))

When its not working ie emacsclient after-make-frame-functions has this value: (indent-bars-setup-and-remove x-dnd-init-frame)

indent-bars-spacing is nil

When its working after-make-frame-functions is (x-dnd-init-frame) and indent-bars-spacing is 4

jdtsmith commented 1 year ago

Interesting. So after-make-frame-functions is not being called. The only reason that is setup is because you are running emacs in daemon mode, isn't that so? What is the value of M-: (daemonp) in your emacs session?

dinkonin commented 1 year ago

(daemonp) returns t when its not working , and nil when it is. I dont explicitly run emacs --daemon or emacs --fg-daemon so I'm assuming emacsclient just starts one if does not find a running emacs instance.

FYI reverting to a commit before 628d42f2044c417217abb3693ce8ecf663a8b39d fixes the issue. Also this package is very nice, its performance for me compared to alternatives is measurably better, thanks for all your work.

jdtsmith commented 1 year ago

I added a small fix for --daemon to ensure the correct buffer is initialized. May not help you; I do not understand why after-make-frame-functions is being set if you are not starting the daemon.

dinkonin commented 1 year ago

After reading a bit about emacslient it seems that actually I'm starting a deamon with the -a '' parameter.

As a special exception, if command is the empty string, then emacsclient starts Emacs in daemon mode (as ‘emacs --daemon’) and then tries connecting again.

dinkonin commented 1 year ago

This patch works for me, am I missing something obvious ?

--- a/indent-bars.el
+++ b/indent-bars.el
@@ -993,7 +993,7 @@ Adapted from `highlight-indentation-mode'."
   :global nil
   :group 'indent-bars
   (if indent-bars-mode
-      (if (daemonp)
+      (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)))
jdtsmith commented 1 year ago

Aha, so you are running --daemon in disguise. I don't have a good way to test --daemon since my version doesn't support it well. Can you explain what your patch does? This code is supposed to protect against indent-bars being initialized and requesting frame information for its display setup, prior to a frame actually existing. Can you think of a way to verify it's doing that?

jdtsmith commented 1 year ago

For now I have reverted the commit that apparently wasn't helping.

dinkonin commented 1 year ago

AFAIK when emacs server creates a new frame it puts the 'client parameter to the frame as you can see here: https://github.com/emacs-mirror/emacs/blob/60e5f212182ca2f41f89a4315075e38433bc8ac0/lisp/server.el#L983C1-L983C1

In my patch I just added a check for it so the hook is added only of emacs is running in daemon mode and has NO active client frame.

jdtsmith commented 1 year ago

OK. So the idea is that --daemon isn't on its own the problem, but just --daemon initializing indent-bars before the first frame? I've re-enabled the check with your suggestion; give a try? I guess we'll have to see if it squashes other --daemon issues people have reported.

dinkonin commented 1 year ago

It works for me. Lets hope that I did not break something. Thank you !