improve ess loading and fix ess-inferior-r-mode-map
bug
#16366
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: this is closely related to #16364, but offers a more comprehensive fix without removing the functionality
This commit is motivated by a bug I encountered wherein
ess
would not load properly (e.g., deferred load triggered when editingfoo.R
) and I'd get an error aboutess-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 foress-site
rather thaness
; the autoloads foress
are such that one could go a long way without ever actually triggeringess-site
, in which case nothing in the:config
block would run. I've changed theuse-package
block to refer toess
, 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 variousess
features. However, this is kind of fragile, as sometimesess
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 REess-inferior-r-mode-map
is one such instance. There was a block that looked like this:and
spacemacs/ess-bind-keys-for-inferior
would try to manipulate two keymaps:inferior-ess-mode-map
andinferior-ess-r-mode-map
. The former is defined iness-inf.el
, but the latter is defined iness-r-mode.el
. Thus, this form would trigger as soon asess-inf.el
was loaded, but in many cases this was beforeess-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 withiness
. This may cause a bit of an overhead wheneveress
is loaded, e.g., when the user first visits filefoo.R
, but the overhead shouldn't be experienced during startup and is probably worth it to have a more robust configuration.Thank you ❤️ for contributing to Spacemacs!
Before you submit this pull request, please ensure it is against the
develop
branch and that you have read the CONTRIBUTING.org file. It contains instructions on how to properly follow our conventions on commit messages, documentation, change log entries and other elements.Don't forget to update CHANGELOG.develop if you want to be mentioned in the release file!
This message should be replaced with a description of your change.
Cheers!