Skip to content

Commit

Permalink
improve ess loading and fix ess-inferior-r-mode-map bug
Browse files Browse the repository at this point in the history
This commit is motivated by a bug I encountered wherein `ess` would not load
properly (e.g., deferred load triggered when editing `foo.R`) and I'd get an
error about `ess-inferior-r-mode-map` being an undefined variable.

In the process of fixing the bug, I realized an opportunity to simplify the way
ess gets autoloaded and configured.

Previously, `use-package` was defined for `ess-site` rather than `ess`; the
autoloads for `ess` are such that one could go a long way without ever actually
triggering `ess-site`, in which case nothing in the `:config` block would run.
I've changed the `use-package` block to refer to `ess`, so that the autoloads
will actually cause things in the `:config` block to fire.

Perhaps because of this, there were a whole bunch of `with-eval-after-load`
blocks under `:init` that attempted to do just-in-time configuration of various
`ess` features. However, this is kind of fragile, as sometimes `ess` gets
refactored in such a way that it only makes since to do this configuration
after *multiple* libraries are loaded. For example, the original bug RE
`ess-inferior-r-mode-map` is one such instance. There was a block that looked
like this:

```
(with-eval-after-load 'ess-inf
(spacemacs/ess-bind-keys-for-inferior))
```

and `spacemacs/ess-bind-keys-for-inferior` would try to manipulate two keymaps:
`inferior-ess-mode-map` and `inferior-ess-r-mode-map`. The former is defined in
`ess-inf.el`, but the latter is defined in `ess-r-mode.el`. Thus, this form
would trigger as soon as `ess-inf.el` was loaded, but in many cases this
was *before* `ess-r-mode.el` had been loaded, which caused the error.

I have streamlined all of this configuration by moving it out of the
`with-eval-after-load` forms in `:init` and putting each directly into the
`:config` block. In order to avoid the sundry dependencies of this code, the
`:config` block now starts with `(require 'ess-site)`, which should trigger
loading of all of the libraries within `ess`. This may cause a bit of an
overhead whenever `ess` is loaded, e.g., when the user first visits file
`foo.R`, but the overhead shouldn't be experienced during startup and is
probably worth it to have a more robust configuration.
  • Loading branch information
dankessler authored and smile13241324 committed May 11, 2024
1 parent 1ce1b0e commit 2fb41e7
Showing 1 changed file with 9 additions and 11 deletions.
20 changes: 9 additions & 11 deletions layers/+lang/ess/packages.el
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
(spacemacs/enable-flycheck 'ess-r-mode))

(defun ess/init-ess ()
(use-package ess-site
(use-package ess
:mode (("\\.sp\\'" . S-mode)
("/R/.*\\.q\\'" . R-mode)
("\\.[qsS]\\'" . S-mode)
Expand Down Expand Up @@ -94,17 +94,15 @@
(add-hook 'inferior-ess-mode-hook
'spacemacs//ess-fix-read-only-inferior-ess-mode)

(with-eval-after-load 'ess-julia
(spacemacs/ess-bind-keys-for-julia))
(with-eval-after-load 'ess-r-mode
(spacemacs/ess-bind-keys-for-r)
(unless (eq ess-r-backend 'lsp)
(spacemacs/declare-prefix-for-mode 'ess-r-mode "mg" "goto")
(define-key ess-doc-map "h" #'ess-display-help-on-object)))
(with-eval-after-load 'ess-inf
(spacemacs/ess-bind-keys-for-inferior))
:config
(define-key ess-mode-map (kbd "<s-return>") #'ess-eval-line))
(require 'ess-site) ; ensure fully loaded for config to work
(define-key ess-mode-map (kbd "<s-return>") #'ess-eval-line)
(spacemacs/ess-bind-keys-for-julia)
(spacemacs/ess-bind-keys-for-r)
(unless (eq ess-r-backend 'lsp)
(spacemacs/declare-prefix-for-mode 'ess-r-mode "mg" "goto")
(define-key ess-doc-map "h" #'ess-display-help-on-object))
(spacemacs/ess-bind-keys-for-inferior))

;; xref integration added with #96ef5a6
(spacemacs|define-jump-handlers ess-mode 'xref-find-definitions))
Expand Down

0 comments on commit 2fb41e7

Please sign in to comment.