search

emacs-ess / ESS

Emacs Speaks Statistics: ESS
https://ess.r-project.org/
GNU General Public License v3.0
623 stars 162 forks source link

Inconsistency in the definition of ess-lisp-directory and ess-dir #1293

Open plantarum opened 6 months ago

plantarum commented 6 months ago

Apologies for this very trivial 'issue'. It's probably only relevant to my unusual use case, where I'm 'installing' ESS (from source) into my .emacs.d/ directory, rather than the usual places (i.e., via elpa, or in /usr/emacs/site-lisp).

The variable ess-lisp-directory is defined in ess-custom.el:

https://github.com/emacs-ess/ESS/blob/d941b25f10716c307f546bdd9cb85d4ac5c1cf38/lisp/ess-custom.el#L137-L140

Using file-name-directory should always return a directory, which in my case is "/home/smithty/.emacs.d2/repos/ess/lisp/" (note the trailing /).

When that string is used to define ess-dir in ess.el:

https://github.com/emacs-ess/ESS/blob/d941b25f10716c307f546bdd9cb85d4ac5c1cf38/lisp/ess.el#L87-L88

it is passed on to file-name-directory again. When file-name-directory receives a directory, it returns the same directory. In this case I think the intention is to return the parent directory of ess-lisp-directory, so this is an error. The consequence is that when I call ess-version I get the following message:

ess-version:  [<unknown>] (loaded from /home/smithty/.emacs.d2/repos/ess/lisp/)

I think this can be corrected by coercing ess-lisp-directory to a directory file name:

(defun ess-version-string ()
  (let* ((ess-dir (file-name-directory
                   (directory-file-name ess-lisp-directory))) ; if(<from source>) the top-level 'ess/'

With this change, I get the correct value from ess-version:

ess-version:  [git: d941b25f10716c307f546bdd9cb85d4ac5c1cf38] (loaded from /home/smithty/.emacs.d2/repos/ess/lisp/)

Since this doesn't actually change the value of ess-lisp-directory, and ess-dir is local to the function ess-version-string, I don't think it should break anything.

I'm a little embarrassed to raise such a trivial issue. Really I just want to make sure I'm installing ESS properly, in the event that I'm then able to contribute anything of more substance in future.

In any case, if this makes sense I could make a pull request.