-
Notifications
You must be signed in to change notification settings - Fork 162
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allows some devtools commands to be run in a compilation-mode buffer. #531
base: master
Are you sure you want to change the base?
Conversation
(ess-r-package-eval-linewise | ||
"devtools::check(%s)\n" "Checking %s" arg | ||
'("" (read-string "Arguments: " "vignettes = FALSE")))) | ||
(let ((actions '("" (read-string "Arguments: " "vignettes = FALSE")))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too much copy pasting. I would add a wrapper which would dispatch to ess-r-package-eval-linewise
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a hard time coming up with a design for such a wrapper -- do you have an idea about how to do it nicely?
lisp/ess-r-package.el
Outdated
"Evalate R expression EXPR in a standalone `compilation-mode' | ||
buffer." | ||
(let* ((procname inferior-ess-r-program-name)) | ||
(ess-r-package-compilation-start |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this extra helper is not warranted. Could be inlined here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the helper could be used to run non-R commands in the project directory. But perhaps it's premature to build out that feature without an actual use case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Let's strive for minimal code for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
(ess-r-package-set-package))) | ||
(pkg-name (ess-r-package-name)) | ||
(default-directory (cdr pkg-info))) | ||
(compilation-start command nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
t
instead of nil would result in compilation-shell-minor-mode which should solve the term signals issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately this also puts it in comint-mode
, which is quite a different experience than the usual compilation-mode
. I don't see an obvious way to get term signal handling outside of comint-mode
-- do you have any inkling of what could be done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does that matter? If compilation mode is based on comint this is how it is.
This commit introduces a `ess-r-package-use-compile-buffer` flag that allows users to specify that `check()`, `test()` and `document()` should be run in a standalone buffer. Signed-off-by: Aaron Jacobs <atheriel@gmail.com>
Hey, sorry for the long silence on this. I've made a minor update to the patch, but I'm not sure how to proceed. |
FYI I have extracted (defun ess-r-eval-in-compile-buffer (expr)
"Evalate R expression EXPR in a standalone `compilation-mode' buffer."
(let* ((procname inferior-ess-r-program-name)
(command (format "%s --slave --no-readline -e \"%s\"" procname expr)))
(compilation-start command nil
(lambda (name-of-mode)
(concat "*" (downcase name-of-mode) "*"))
ess-r-error-regexp-alist))) I use it for rendering markdown documents: (defun my-ess-rmarkdown-render ()
(interactive)
(let ((cmd (format "rmarkdown::render('%s')" buffer-file-name)))
(ess-r-eval-in-compile-buffer cmd))) I quite like that this uses a separate process at each time rendering. I haven't tried it yet for devtools commands but I think we should explore compilation-mode further after the next release. |
@atheriel sorry for the late reply. We will merge this after the release in September. A bit more thinking is needed here. |
Yes, I appreciate that. I'd like to get back to it. One thing I hadn't considered (but is pertinent to my own use cases) is how to ensure that this will interoperate with |
What's the status of this? Are we implementing it for 19.04? |
I don't think so. But I'd still like to integrate compilation buffers in ESS at some point. |
I'm still a big fan of this approach, although there are a few outstanding issues mentioned in the thread: (1) Rendering of escape characters. As well as any actual design questions. Any thoughts on these? |
I had some very concrete thoughts back then but didn't have time to get to it. And now I am even more busy to have a look at this. One thing for sure, the functionality should be brought in once we have the local cache for prog + params which is also needed for restarts. |
This would be a nice feature! Any update? |
This is a proof-of-concept implementation of the feature discussed in #528. I've only implemented
check()
,test()
, anddocument()
for now, and obviously the documentation could use some work as well.How does this general approach look?
(Known issue: compilation mode buffers do not render the shell-escaped characters produced by
devtools::test()
correctly.)