Skip to content
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

Update ess layer readme by common configuration options #16296

Closed
wants to merge 2 commits into from

Conversation

rommeswi
Copy link
Contributor

The commit adds two configuration options to the readme.

The first is a way to stop ESS from asking what the R working directory should be.
The second is a way to stop ESS from indenting standard R comments starting with '#' by 40 spaces.

I think a lot of ESS users will be interested in knowing these configuration options.
It took me a while to find them on the web, therefore I think it would be useful to include them in the layer readme.
For the 40 space automatic indentation, I even thought it was a bug.

@ryanprior
Copy link
Contributor

This looks helpful, thank you!

It's interesting that CircleCI is failing on "cat: /tmp/changed_files: No such file or directory" - I'm guessing that's from the end of this script: https://github.com/syl20bnr/spacemacs/blob/develop/.circleci/select_pr_changed#L23

As an experiment @rommeswi, will you make a small update to an .el file and see if that allows the documentation test to run?
@JAremko do you have any idea why it's failing like that?

@JAremko
Copy link
Collaborator

JAremko commented Feb 27, 2024

@ryanprior looks like one of the involved services glitched.

Clarifies the code greatly.
@rommeswi
Copy link
Contributor Author

Circleci passed immediately with the change in the .el file (before it failed on the first attempt). However, it was also makerd as passed right before I added the other commit. So I am not sure if something has been fixed, it was just a glitch, or indeed editing an .el file helps.

@downward-funarg
Copy link
Contributor

It should be noted that setting ess-ask-for-ess-directory true causes the REPL to open in the project home directory, not the file directory. See emacs-ess/ESS#461

@rommeswi
Copy link
Contributor Author

rommeswi commented Mar 4, 2024

I think that "feature" no longer exists: Ess Manual
I also just confirmed that on emacs itself. I started the process in a subdirectory of a git folder and R used setwd on the subdirectory, not the project directory.

(Also, I think you meant "false" instead of "true", or? If the value is set to true then it will ask instead of choosing the home directory.)

@downward-funarg
Copy link
Contributor

Sorry, yes I meant "false". But I can't reproduce your finding:

(setq-default ess-ask-for-ess-directory nil)
(make-directory "~/test-R/test" t)
(magit-init "~/test-R")
(find-file "~/test-R/test/repl-test.R")
(save-buffer)
(spacemacs/ess-start-repl)

opens in ~/test-R.

@rommeswi
Copy link
Contributor Author

rommeswi commented Mar 5, 2024

Strange. I just tried again. I am in a file inside a git repository that also shows up in my home buffer. My modeline shows me that it indeed recognizes that it is inside a git repo. No matter whether I then run M-x R or M-X spacemacs/ess-start-repl, I always get setwd to the folder where the R file is located, not the git repo.

The thread that you linked is 6 years old and it would be undocumented behavior (see the link I posted above). Are you sure you are on a current version of ESS and emacs? I could also imagine that it matters in which folder you execute the lisp code you posted above.

In any case, if you have any additions to make to the readme, please go ahead and create a separate pull request.

@downward-funarg
Copy link
Contributor

This is indeed strange, I just tried with a fresh clone of spacemacs on a different emacs version and OS (30.0.50, Ubuntu 20.04; my initial system was emacs-mac-port on macOS Sonoma) and got the same behavior that I reported.

But I guess this is not really spacemacs' problem, except insofar as this behavior is prevalent enough to mention in the readme - would be useful if others can give their experience to decide about that.

@downward-funarg
Copy link
Contributor

@rommeswi Actually, opening in the project directory is expected behavior and is documented. From the documentation for ess-directory:

If ess-startup-directory-function is nil, then the current project as returned by project-current is used.

And it's in the ESS manual:

ESS[R]: Add support for R projects and start R by default in the project folder.

I wonder why your setup doesn't do this! Anyway, the docstring also suggests setting (setq ess-startup-directory 'default-directory) to disable this behavior, which works for me.

@rommeswi
Copy link
Contributor Author

rommeswi commented Mar 7, 2024

@downward-funarg please go ahead and create a pull request with an updated readme that properly documents behavior in your view. I don't think this is a productive discussion. The behavior on my system that I tried to enter into the readme is consistent with what is linked in the manual link I provided above.

@downward-funarg
Copy link
Contributor

I'm commenting on your pull request because your contribution would add an option which might have detrimental or unexpected effects. Feel free to ignore my comments, I suppose, but this information is relevant when considering if this change should be merged.

I understand that your system behaves as you say it does, but so does mine, and if you'll read the manual link which I also provided, you will see that the behavior I describe is expected for the current ESS version.

@rommeswi
Copy link
Contributor Author

rommeswi commented Mar 7, 2024

If you can comment, I'm sure you can also update the readme. Go ahead. I'm closing this.

@rommeswi rommeswi closed this Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants