ESS should not affect project definition

[christophe-gouel]

Hi,

I don't think this is such a good idea to have ess redefining project root. I am thinking about this line in ess-r-mode.el:

  (add-hook 'project-find-functions #'ess-r-project nil 'local)

It redefines project root as a folder containing files .Rprofile, .Rproj, or DESCRIPTION.

This has several consequences. Project.el commands behave differently if the current buffer is a R file or if it is a dired buffer visiting the same folder or if it is a Python file. This is really an unexpected behavior for a project.el user! If each major mode does this, things can be a real mess.

In addition, .Rprofile files can show up in subfolders, which completely breaks the usefulness of project.el. It took me quite some time to understand why project.el was completely failing for me with R buffers, while projectile worked great. In my case, I have a .Rprofile file in my root project folder, but often also in my script folders (with simply source("../.Rprofile", chdir = TRUE) inside). This is because if I want to run the R files as script without having to mess up with the working directory, I need to load the profile from there (typical use case is the background job approach of RStudio or another language running the R files).

[lionel-]

I agree that the behaviour with .Rprofile can cause some puzzling moments because that file does not necessarily represent a project.

Regarding hooking into project.el, what would be your recommendation? Are there existing practices regarding hooking up into it in a polite manner? Would you be willing to compile some examples from e.g. python or julia modes?

The main idea of making it the default is to steer users towards workflow where state (e.g. current dir) is encoded in the code instead of being created by REPL interaction. That said the audience of a project like ESS is increasingly different from the audience of RStudio, so it may not matter as much if we require users to opt into the good default.

But we also have to consider that changing the default now after that many years would probably annoy lots of users.

[christophe-gouel]

Thanks for responding quickly. Let me take a few days to look at other projects to see how they do this, if they do it.

[mmaechler]

I agree that the automatic "project setting" can be very unpleasant. As R Core maintainer, and also when teaching advanced R, I do want to quickly browse package folders, for illustration, exploration, showing examples, etc. I hate when ESS is doing things in such circumstances.

Even worse, when I am in a *shell* buffer, cd'ing and grep'ing around in a "top" folder containing several (sometimes many) unpacked packages as subfolder. I do not want ESS to do anything and definitely don't want it to "take over" my *shell* buffer as being related to my current R project. ... notably as this also makes moving around ("cd'ing") in the shell quite slow and hence inconvenient and confusing.

[christophe-gouel]

I have done some checks:

• For python mode (included in emacs), there is nothing about projects, which is expected from a core package that should be careful about not stepping on another core package.
• For elpy, there is project management built into the package. So, you have keybindings for looking at files in the project or looking for regexp (but different from project.el and projectile bindings). There is no redefinition of project.el hook and elpy relies on projectile for looking for the project root. This is a weird strategy for me: I see no reason for each major mode to redefine project-specific functions.
• For julia mode, there is nothing related to projects.

So, my understanding is that the norm is for programming major modes to leave the definition of what is a project left to project packages.

I don't think there should be too much concern about changing the default after many years. This is a bad default, which goes against emacs philosophy, like setting keybindings to the user-reserved keybindings, a practice abandoned in ess after many years.

It is always possible to add explanations about the change in the documentation and release notes. If some are missing the old behavior, they can always add back the hook to their configuration file. However, to have a behavior consistent with what is expected from a project (meaning not changing the behavior when in an R file or in dired), the hook should not be mode-dependent. Something like the line below should do the job for those interested by the old behavior:

(add-hook 'project-find-functions #'ess-r-project 'append)

[lionel-]

Just to be clear, I don't think it's a bad default, I think it's a very good one. And I also think that hooking into things is part of the Emacs philosophy. I tend to consider that adding a backend to project.el is comparable to, e.g., adding a backend to company-mode. Sure from time to time we get surprising company completions because of this, but overall the ecosystem benefits from these implicit integrations.

We should probably add a customisable variable that instructs ESS whether to hook into project.el, but I think it should remain true by default. At least it'd be easy to disable.

At the same time we should be careful about our criteria for determining an R project to avoid any confusing behaviour. So probably the .Rprofile detection should be optional, and maybe disabled by default.

[christophe-gouel]

Just to be clear, I don't think it's a bad default, I think it's a very good one. And I also think that hooking into things is part of the Emacs philosophy. I tend to consider that adding a backend to project.el is comparable to, e.g., adding a backend to company-mode. Sure from time to time we get surprising company completions because of this, but overall the ecosystem benefits from these implicit integrations.

I apologize for the strong wording.

Note, though, that in what is currently done this is not adding a backend to project.el, but replacing the backend, which is a bit different, since this increases the side effects. Hence, the append in what I propose.

The reason why I consider it a bad default is because it leads to a mode-specific behavior for project.el, which is something completely unexpected (contrary to company). That's why I think that project issues are best left outside of major modes. If I am in a Rmarkdown or a Quarto file, I get a different behavior of project.el if I am in the markdown part (standard project-find-file) or if I am in the R part. I have a lot of difficulty to see how this could make sense.

Obviously, the problem is created by the detection of .Rprofile, and a lot of issues would be removed by disabling it, but my broader point remains.

If people wants something more flexible than project.el regarding project detection, they can use projectile (even just for the detection side which is automatically added to the project hook), which already include something for R projects (https://docs.projectile.mx/projectile/projects.html#supported-project-types).

[lionel-]

Hence, the append in what I propose.

IIUC you're proposing to use this hook precedence so that it doesn't override user configuration?

The depth defaults to 0 and for backward compatibility when depth
is a non-nil symbol it is interpreted as a depth of 90

The reason why I consider it a bad default is because it leads to a mode-specific behavior for project.el, which is something completely unexpected (contrary to company). That's why I think that project issues are best left outside of major modes. If I am in a Rmarkdown or a Quarto file, I get a different behavior of project.el if I am in the markdown part (standard project-find-file) or if I am in the R part. I have a lot of difficulty to see how this could make sense.

I may be missing something. Isn't our project finder independent of the major-mode?

If people wants something more flexible than project.el regarding project detection, they can use projectile

I don't think we want to offload our definition of an R project in Emacs to projectile ;)

[christophe-gouel]

IIUC you're proposing to use this hook precedence so that it doesn't override user configuration?

Yes, I do not see a reason to override any existing configuration. You want to add a way to detect a project, not to define the one and true way!

I may be missing something. Isn't our project finder independent of the major-mode?

Yes: if the buffer is an R file or an R shell, I get the following from the help on project-find-functions:

project-find-functions is a variable defined in ‘project.el’.

Its value is (ess-r-project t)
Local in buffer tidy_baci.R; global value is 
(project-projectile project-try-vc)

Special hook to find the project containing a given directory.
Each functions on this hook is called in turn with one
argument, the directory in which to look, and should return
either nil to mean that it is not applicable, or a project instance.
The exact form of the project instance is up to each respective
function; the only practical limitation is to use values that
‘cl-defmethod’ can dispatch on, like a cons cell, or a list, or a
CL struct.

  This variable may be risky if used as a file-local variable.

If the buffer is something else, but in the same folder, I get my default project-find-functions. This is what is annoying me. On the other hand, this is kind of inevitable since it should not be the business of a major mode to redefine the project-find-functions everywhere.

[lionel-]

In summary:

• We want to remove the 'local argument that sets the hook to be buffer local. IIUC this is what prevents the other backends to be picked up, and also what makes the project backends different across modes.

• We possibly want to add a positive depth, but I'm a bit concerned that it puts our backend on the end of the list after projectile and vc, though that may be fine.

[christophe-gouel]

• We want to remove the 'local argument that sets the hook to be buffer local. IIUC this is what prevents the other backends to be picked up, and also what makes the project backends different across modes.

Yes, this ensures a consistent behavior. The big caveat is that it implies ess has, by default, effect on project backends everywhere. My opinion is that this is not ess business, but we disagree on this. Hopefully, this can be dealt with the last 2 points.

• We possibly want to add a positive depth, but I'm a bit concerned that it puts our backend on the end of the list after projectile and vc, though that may be fine.

Regarding the depth, I cannot be of any help, it really exceeds my understanding of hooks.

Don't forget also:

• We want to remove the detection of .Rprofile, since it may lead subfolders to be detected as the root folder.
• We want to add a custom variable for those like me who do not want to have their project backends affected at all.

[mmaechler]

* We want to add a custom variable for those like me who do not want to have their project backends affected at all.

Yes, PLEASE .... personally I still think this custom variable (if it had several _levels" of increasing thoroughness) should have a default that is less agressively "taking over" management of standard ESS buffers than now; notably by default, none of the *shell* buffers should by default be influenced by automatically detected R projects --- which I almost never care about, because I am looking into many more R packages (and possibly other projects) than I work with.

[christophe-gouel]

@mmaechler, if you want to get rid of this behavior now, add the following to your ess configuration (inside a use-package block):

:hook
(ess-r-mode . (lambda()
          (make-local-variable 'project-find-functions)
          (setq project-find-functions '(project-try-vc))))

Do also the same for inferior-ess-mode.