Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

magit-emacsclient-executable: discover emacsclient name via pattern match #1192

Closed
wants to merge 1 commit into from

4 participants

@mgalgs

It's possible to configure Emacs with --program-suffix=-suffix which
results in the emacsclient executable being named
emacsclient-suffix. For example, it's not uncommon to track the Emacs
development repository and configure Emacs with --program-suffix=-git or
--program-suffix=-bzr.

The current search patterns all assume Emacs wasn't configured with a
program suffix. Fix this by searching the invocation-directory for any
program that matches the regexp: emacsclient.*.

@mgalgs mgalgs magit-emacsclient-executable: discover emacsclient name via pattern m…
…atch

It's possible to configure Emacs with --program-suffix=-suffix which
results in the emacsclient executable being named
emacsclient-suffix. For example, it's not uncommon to track the Emacs
development repository and configure Emacs with --program-suffix=-git or
--program-suffix=-bzr.

The current search patterns all assume Emacs wasn't configured with a
program suffix. Fix this by searching the `invocation-directory' for any
program that matches the regexp: `emacsclient.*'.
8c5cdbd
@tarsius
Owner

Using the lowest version found seems wrong. We could use version just defined above, or check the Emacs executable for a version suffix, that should always be the same, right? The suffix can be anything, not just integers and periods, right?

@tarsius tarsius added this to the 2.1.0 milestone
@PureAbstract

We've had problems with this before (TBH, I think the existing code is already trying to be too clever; I accept part of the blame for that :)

I'd argue that folks who are building emacs from source are probably quite capable of setting magit-emacsclient-executable correctly by hand.

@mgalgs

@tarsius

Using the lowest version found seems wrong. We could use version just defined above, or check the Emacs executable for a version suffix, that should always be the same, right?

I think I'm missing something, I just take the first thing in the directory where the current emacs executable resides that starts with emacsclient.

As far as the version suffix goes, there's another number that I couldn't find an elisp variable for. For example, my versioned executable name is emacs-24.3.50-git, and then I have a symlink emacs-git that points to that. That's the default build output when you use --program-suffix.

The suffix can be anything, not just integers and periods, right?

Yes, it can be anything.

@PureAbstract

I'd argue that folks who are building emacs from source are probably quite capable of setting magit-emacsclient-executable correctly by hand.

Yes, but just because they're capable doesn't mean they should be inconvenienced more than other users... Also, I don't run emacs from git on all the machines I use, but I do share my magit configuration, so I would be inconvenienced further to special-case my config for each machine (or do a search like this patch does). I'm not quite clear on what's "too clever" here... I agree it's not beautiful but I don't see anything particularly risky.

Actually, maybe a modified version of this patch could simplify things. It seems like we could actually replace all of the other cases with this single case if we really wanted to... Is there ever a case where that would be incorrect?

@PureAbstract

@mgalgs

I think I'm missing something, I just take the first thing in the directory where the current emacs executable resides that starts with emacsclient.

Which - unless I'm missing something - doesn't solve your stated use-case. Unless you tell it not to, directory-files returns a sorted list, so emacsclient will appear before emacsclient-git or emacsclient-bzr, so you'll get emacsclient. And I don't think that's what you want?

Is there ever a case where that would be incorrect?

Yes; most obviously the case where emacsclient doesn't live in the same directory as the emacs executable. See #862.

@mbunkus

On Debian/Ubuntu you can easily have emacs, emacs23, emacs24, emacsclient, emacsclient23 and emacsclient24 in /usr/bin just by installing the emacs23 and emacs24 packages. And which versions of those two emacs and emacsclient refer to can be selected by the user. Simply using the first thing looking like emacsclient* is not going to work reliably here.

@mgalgs

I think I'm missing something, I just take the first thing in the directory where the current emacs executable resides that starts with emacsclient.

Which - unless I'm missing something - doesn't solve your stated use-case. Unless you tell it not to, directory-files returns a sorted list, so emacsclient will appear before emacsclient-git or emacsclient-bzr, so you'll get emacsclient. And I don't think that's what you want?

Ah yes, I always install to a separate directory so I don't have that problem. Makes sense now.

Is there ever a case where that would be incorrect?

Yes; most obviously the case where emacsclient doesn't live in the same directory as the emacs executable. See #862.

Indeed, thanks for the example.

Hmm... I see two options here: (1) Figure out how to sort directory-files by versioned executable name, or (2) put this at the end of the list of patterns we search for, as a sort of "last resort".

@tarsius tarsius added the 21 wontadd label
@tarsius
Owner

This is a collection of hacks on top of hacks. Every time we change something we risk breaking it again for somebody else. So unless a user shows up saying he cannot get it to work, I would rather not touch this.

The reason we need this in the first place is that it turned out that using the emacsclient is rather advanced emacs usage. Many users have never attempted to use the emacsclient, but magit forces them to do so. So we in turn are forced to work around missing or bad configuration, or we would end up with an endless stream of bug reports.

This was very unpleasant for me, so again if it isn't broken I don't want to fix it. There will be one more makeover but that will happen at a well defined time when everything else has been taken care of.

@tarsius tarsius closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 7, 2014
  1. @mgalgs

    magit-emacsclient-executable: discover emacsclient name via pattern m…

    mgalgs authored
    …atch
    
    It's possible to configure Emacs with --program-suffix=-suffix which
    results in the emacsclient executable being named
    emacsclient-suffix. For example, it's not uncommon to track the Emacs
    development repository and configure Emacs with --program-suffix=-git or
    --program-suffix=-bzr.
    
    The current search patterns all assume Emacs wasn't configured with a
    program suffix. Fix this by searching the `invocation-directory' for any
    program that matches the regexp: `emacsclient.*'.
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 2 deletions.
  1. +7 −2 magit.el
View
9 magit.el
@@ -230,13 +230,18 @@ tramp to connect to servers with ancient Git versions."
(shell-quote-argument
(let ((version (format "%s.%s"
emacs-major-version
- emacs-minor-version)))
+ emacs-minor-version))
+ (emacsclient-name (car (directory-files invocation-directory
+ nil
+ "emacsclient.*"))))
(or (let ((exec-path (list (expand-file-name "bin" invocation-directory)
invocation-directory)))
- (or (executable-find (format "emacsclient-%s" version))
+ (or (executable-find emacsclient-name)
+ (executable-find (format "emacsclient-%s" version))
(executable-find (format "emacsclient-%s.exe" version))
(executable-find "emacsclient")
(executable-find "emacsclient.exe")))
+ (executable-find emacsclient-name)
(executable-find (format "emacsclient-%s" version))
(executable-find (format "emacsclient-%s.exe" version))
(executable-find "emacsclient")
Something went wrong with that request. Please try again.