magit / with-editor

Use the Emacsclient as the $EDITOR of child processes
http://magit.vc/manual/with-editor
GNU General Public License v3.0
188 stars 44 forks source link

Support of Emacs suffix #132

Open yalfg opened 3 weeks ago

yalfg commented 3 weeks ago

Hi,

When Emacs is compiled with a suffix (configure option "--program-suffix=-fsf" in my case), with-editor cannot find the matching emacsclient. I work-around the issue with the following code in my init:

  (when (string-match "emacs\\(.+\\)" invocation-name)
    (let* ((suffix (match-string 1 invocation-name))
           (client (executable-find (concat "emacsclient" suffix))))
      (customize-set-variable 'with-editor-emacsclient-executable client
                              "Can't be found automatically with a suffix")))

FYI, I'm using the suffix to be able to co-install my distribution version of Debian and a manually compiled upstream Debian, for which I use the "-fsf" suffix. In this configuration, with-editor picks the distribution emacsclient instead of the matching one with same suffix.

Here's the debug output FYI:

with-editor: /home/user/.emacs.d/elpa/with-editor-3.4.2/with-editor.el
emacs: /usr/local/bin/emacs-fsf (29.4)
system:
  system-type: gnu/linux
  system-configuration: x86_64-pc-linux-gnu
  system-configuration-options: --prefix=/usr/local/stow/emacs-29.4 --program-suffix=-fsf --with-json --with-tree-sitter --with-native-compilation --with-file-notification=yes --with-xwidgets 'CFLAGS=-O2 -march=native'
server:
  server-running-p: t
  server-process: #<process server8359>
  server-use-tcp: nil
  server-name: server8359
  server-socket-dir: /run/user/1000/emacs
    server
    server8359
  server-auth-dir: ~/.emacs.d/server/
    WARNING: not an accessible directory
with-editor-emacsclient-executable:
 value:   /usr/local/bin/emacsclient-fsf (29.4)
 default: /usr/local/bin/emacsclient-fsf (29.4)
 funcall: /usr/bin/emacsclient.emacs (29.4)
path:
  $PATH:     (/home/user/.opam/4.11.2/bin /home/user/bin /home/user/.cargo/bin /usr/local/sbin /usr/local/bin /snap/bin /usr/sbin /usr/bin /usr/games /usr/local/games /home/user/.local/bin)
  exec-path: (/home/user/.opam/4.11.2/bin /home/user/bin /home/user/.cargo/bin /usr/local/sbin /usr/local/bin /snap/bin /usr/sbin /usr/bin /usr/games /usr/local/games /home/user/.local/bin /usr/local/stow/emacs-29.4/libexec/emacs/29.4/x86_64-pc-linux-gnu)
  with-editor-emacsclient-path:
    /usr/local/stow/emacs-fsf/bin (t)
      /usr/local/stow/emacs-fsf/bin/emacsclient-fsf (29.4)
    /home/user/.opam/4.11.2/bin (t)
    /home/user/bin (t)
      /home/user/bin/myemacsclient (29.4)
    /home/user/.cargo/bin (t)
    /usr/local/sbin (t)
    /usr/local/bin (t)
      /usr/local/bin/emacsclient-fsf (29.4)
    /snap/bin (nil)
    /usr/sbin (t)
    /usr/bin (t)
      /usr/bin/emacsclient (29.4)
      /usr/bin/emacsclient.emacs (29.4)
    /usr/games (t)
    /usr/local/games (t)
    /home/user/.local/bin (t)
    /usr/local/stow/emacs-29.4/libexec/emacs/29.4/x86_64-pc-linux-gnu (t)

And by the way, thanks for both Magit and with-editor ;)

tarsius commented 3 weeks ago

This would address the issue, I think, but you would still have to set the new variable, and I don't currently have the time to make sure this wouldn't cause any breakage and that there isn't a better way that would not require that users set that variable.

@@ -111,6 +111,9 @@ (defun with-editor-locate-emacsclient ()
 current Emacs instance failed.  For more information
 please see https://github.com/magit/magit/wiki/Emacsclient."))))

+(defvar with-editor-invocation-suffixes
+  (list "-snapshot" ".emacs-snapshot"))
+
 (defun with-editor-locate-emacsclient-1 (path depth)
   (let* ((version-lst (cl-subseq (split-string emacs-version "\\.") 0 depth))
          (version-reg (concat "^" (string-join version-lst "\\."))))
@@ -128,7 +131,7 @@ (defun with-editor-locate-emacsclient-1 (path depth)
                               (setq v (string-join (reverse v) "."))
                               (list v (concat "-" v) (concat ".emacs" v)))
                             (reverse version-lst))
-                 (list "" "-snapshot" ".emacs-snapshot")))
+                 (cons "" with-editor-invocation-suffixes)))
          (lambda (exec)
            (ignore-errors
              (string-match-p version-reg
yalfg commented 3 weeks ago

Thanks! I've just compiled the Emacs 30 pre-release and besides a suffix (the only thing I've used) it's possible to add a prefix and even a general modifier. Extracted from the configure help:

Program names:
  --program-prefix=PREFIX            prepend PREFIX to installed program names
  --program-suffix=SUFFIX            append SUFFIX to installed program names
  --program-transform-name=PROGRAM   run sed PROGRAM on installed program names

So trying to second guess all this and in particular the sed option seems too brittle. And it may not be much used too.

There's a variable emacsclient-program-name (new in Emacs 30), which seems perfect. Unfortunately it doesn't take into account the above transformation: on my build with the -fsf suffix its value is still emacsclient and not emacsclient-fsf as it should. So maybe it would be best to take this upstream, and make sure this variable reflects the actual client name? Then there would be a simple and reliable way from then on, and the current heuristics can remain for older Emacsen?

tarsius commented 3 weeks ago

New work is arriving at an unsustainable rate right now, so I won't be able to deal with this for the foreseeable future.

yalfg commented 3 weeks ago

Not a problem at all, I actually forgot about this and noticed it again while doing a clean-up pass on my init.el. And nobody else mentioned it before. So it can definitely wait some ;)

I submitted an upstream bug on the content of emacsclient-program-name. A proper value would really be the nicest way to deal with this: why doing fancy heuristics when Emacs should be able to tell us the right value? I'll update here if there's some follow-up (I guess it'll also be low prio for upstream. But it may be an easier fix there. We'll see).

Thanks

yalfg commented 1 week ago

FYI, now covered by upstream Emacs bug #74157. But not as simple as it may seem, so too late for Emacs 30 and to be improved in version 31.