Add support for project-level configuration

[wurli]

Addresses #108.

This PR is somewhat a work in progress with fairly minimal testing - for now I'm really just looking for a sense check on the approach I've taken.

New features

This adds a mapping for the pipe which may either insert |> or %>%, depending on whether an .Rproj file is present and contains the field UseNativePipeOperator. The .Rproj-awareness can be configured using config.rproj_prioritise; for the pipe operator it is on by default.

All changes

• The filetype for .Rproj has been changed to dcf (was yaml)

• New utils.read_dcf() to read .Rproj files

• New module r.rproj for finding/parsing .Rproj files:

  • apply_settings({config} [, {force}, {file}]) updates the config table using a .Rproj file if one is present. This respects config.rproj_prioritise.

  • parse([{file}]) finds a .Rproj file if one is present in the current working directory, and parses it into a dictionary of name/value pairs. NB, I considered recursively searching upwards through the directory tree to find a .Rproj file but decided against it.

• New config.rproj_setup([{force}, {file}]) for applying .Rproj configuration to the current session. I was a bit torn about where this function should live, but the config module seemed most appropriate given how existing code is structured.

• New config options:

  • config.pipe = true

  • config.pipe_map = "<localleader>m"

  • use_native_pipe = true

  • config.rproj_prioritise = { "use_native_pipe" }

[jalvesaq]

From your description, everything seems good to me, but perhaps the first three options could be only two:

• config.pipe = "native" (default, with "no" and "magrittr" as other valid options)
• config.pipe_map = "<LocalLeader>m"

[wurli]

I agree the interface could (should?) be simplified. After a bit more thought, I think my preference would be:

• Use config.pipe_type = "auto" (or "native" or "magrittr"). The auto option would set the magrittr pipe if the current R version is < 4.1, the native pipe otherwise. All 3 options would have lower priority than the .Rprofile setting if we also had config.rproj_prioritise = { "pipe_type" }.

  • Only misgiving about the auto option is that it might make the overall interface too complicated... If you think so then maybe just native and magrittr.

• Use config.pipe_map = nil to disable the pipe mapping

• Supersede config.assign = false in favour of config.assign_map = nil to make the interface uniform

[jalvesaq]

I agree with the auto option. But the nil value doesn't work for the function validate_user_opts() because both an option with value nil and an inexistent option give the same result (false) and, then, the warning "Unrecognized option". That's why no option has nil as its default value. We only have nil as the value of hook members.

[wurli]

I see - thanks for explaining. How would you feel about using another sentinel to indicate no mapping, e.g. {} or a special string like "none"?

I think my aversion to combining config.pipe and config.use_native_pipe is that the .Rproj setting should have higher precedence than use_native_pipe, but lower than pipe, so IMO it would make it hard to understand how the config.rproj_prioritise option is interacting with the other settings.

[jalvesaq]

What about the options for config.pipe being "Rproj" (default), "native", (default if there is no .Rproj in the current directory or the directory above), "no" and "magrittr"?

[wurli]

To be honest it still doesn't feel like the optimum solution to me 🤔 E.g. it then wouldn't be possible to then have magrittr as default, possibly overridden by .Rproj. Plus, if support for more .Rproj fields is added, I think it'd be nice to keep them all configured in the same place so users can quickly see how R.nvim behaviour might vary between projects.

I think I still find more options to clearer en balance. Maybe it'd be more palatable to lean into subtables:

config = {
    -- ...
    assign = {
        create_keymap = true,
        keymap = "<M-->"
    },
    pipe = {
        create_keymap = true,
        keymap = "<localleader>m",
        version = "auto",
        -- A .Rproj option could go here, or (I think my preference) we 
        -- could collect them all together in `config.rproj_prioritise`
        -- prioritise_rproj = true
    },
    -- ...
}

Side note: I used 'keymap' instead of 'mapping' here because I think it could be considered a little clearer given the naming scheme used elsewhere, e.g. vim.keymap.set(), vim.api.nvim_set_keymap().

[jalvesaq]

t then wouldn't be possible to then have magrittr as default, possibly overridden by .Rproj

You are right!

I like the pipe option as a table. That way, it's a single option.

[wurli]

@jalvesaq I think this is ready for review now 😃

Still only adds support for the pipe operator, but as it's already a fairly big PR I thought best to add any extra .Rproj-affected features later.

[jalvesaq]

One observation before trying the code: you call require("r.config").rproj_setup() in all of our 5 filetypes right after real_setup(), so, perhaps the function could instead be called just once within real_setup().

Note: I never worked with .Rproj, so I will mostly look if there are any new bugs in old features.

[jalvesaq]

Nice refactor of validate_user_opts() as apply_user_opts()!
I noted only one mistake: in valid_types you named external_term as external_type.

[jalvesaq]

Did you try in your init.lua different values for 'autochdir' and, then, start nvim in one directory and edit an R file that is in another directory? Will the .Rproj file be found?

vim.o.autochdir = true
vim.o.autochdir = false

And what about different values for the R.nvim config option setwd?

As I said, I never worked with .Rproj, but if RStudio looks for a .Rproj file in the parent directories, users will soon request that R.nvim do the same.

[jalvesaq]

Looking at the changes that you have made to assign, I realized that there is no need to use two options. There is no need to have assign (true or false). If assign_keymap is "", then, don't create the map at all. I can make this change after your pull request is merged to avoid merge conflicts.

[jalvesaq]

In two or three hours from now, I'll try to push some commits into your pull request...

[wurli]

Thanks! Sorry for the late response - been a bit busy since Sunday but done a little work since then in light of your comments. Let me try and push those in the next hour or two, I'll let you know once done so you can go ahead with any edits you want to make.

[wurli]

Did you try in your init.lua different values for 'autochdir' and, then, start nvim in one directory and edit an R file that is in another directory? Will the .Rproj file be found?

vim.o.autochdir = true
vim.o.autochdir = false

And what about different values for the R.nvim config option setwd?

As I said, I never worked with .Rproj, but if RStudio looks for a .Rproj file in the parent directories, users will soon request that R.nvim do the same.

This is a good point. RStudio will ignore .Rproj files unless you tell RStudio to open to that project - i.e. to directory where the .Rproj file exists (most users would usually open RStudio by double-clicking the .Rproj file). In other words, .Rproj settings will only be applied if you start the session in a working directory where a .Rproj file exists.

That said, I think searching upwards would likely only make life easier for people, so happy to add that in anyway.

[wurli]

Hi @jalvesaq, please go ahead with any extra commits you want to make 👍 If you revert assign.keymap and assign.create_keymap to just assign_keymap I might also do similar with the pipe subtable.

Incidentally, could we rename assign_keymap to assignment_keymap? I keep reading 'assign' as a verb - I think the noun form is a bit clearer.

[jalvesaq]

Incidentally, could we rename assign_keymap to assignment_keymap? I keep reading 'assign' as a verb - I think the noun form is a bit clearer.

Yes, we can.

[jalvesaq]

Done! Now there is a single option related to the assignment operator: assignment_keymap.

[wurli]

@jalvesaq FYI I'm still working on this, just quit busy currently, but would like to get it right before merging. Will keep you posted 😃

[jalvesaq]

@PMassicotte noted that <LocalLeader>m is the key binding for sending lines in a Vim motion (jalvesaq/zotcite#77 (comment)).

Another problem: most frequently, the pipe operator is followed by a line break, but, if using the key binding to insert the pipe operator, we will get a trailing space in the pipe operator's line after pressing the <Enter> key.

[wurli]

I've made a few changes:

• The .Rproj file is now searched for on a 'per-buffer' basis, and options are recorded using nvim_buf_set_var(). This means that we can have the pipe keymap insert |> for one script, and %>% for another, as dictated by the corresponding .Rproj files. autochdir has no effect on the behaviour.

• I've changed the default pipe keymap to <localleader>, to avoid the conflict with sending lines in a motion.

Regarding the trailing spaces, I previously wasn't too bothered about this. As far as I'm aware, very few R users care about trailing whitespace - although {lintr} will complain about it. If we want to get rid of it, some options are:

1. Remove the trailing whitespace from the insertion, e.g. insert |> instead of |> . I'd prefer to avoid this option - I think the second version with trailing whitespace is the lesser of two evils when compared to a mapping which doesn't insert the right amount of whitespace for inline pipes.

2. Set up an autocommand to delete trailing whitespace if a newline is inserted right after a pipe. I had a crack at this using nvim_create_autocmd("InsertCharPre", ...), but I couldn't get it working since this doesn't trigger on carriage return ☹️

Any thoughts?

[wurli]

@jalvesaq all look okay now? I found a way to strip trailing whitespace if a new line is inserted directly after a pipe, basically by temporarily remapping <CR> and <C-j> after a pipe is inserted, and removing the keymappings if anything but <CR> or <C-j> is entered after the pipe.

[jalvesaq]

I updated the keybinding in the README to <LocalLeader>, and added the documentation to doc/R.nvim.txt (I copied some text from what you have written in the README and in lua/r/rproj.lua). I guess everything is OK now.

[wurli]

Thank you! 🙏