R-nvim / R.nvim

Neovim plugin to edit R files
GNU General Public License v3.0
128 stars 15 forks source link

add nvimpager=vertical option, split help doc vertically. See #117 #118

Closed cswingle closed 2 months ago

cswingle commented 2 months ago

Adds "vertical" option to nvimpager, which causes help documentation to open in a vertical split instead of the default horizontal split.

Very lightly tested...

wurli commented 2 months ago

I'd suggest split_h and split_v instead of split and vertical? Would mean making superseding split in favour of split_h, and maybe deprecating it after a while.

jalvesaq commented 2 months ago

You could use elseif to avoid increasing the indentation of the code.

jalvesaq commented 2 months ago

We can delete this:

    if
        not config.nvimpager:find("tab")
        and not config.nvimpager:find("split")
        and not config.nvimpager:find("float")
        and not config.nvimpager:find("no")
    then
        warn(
            'Invalid `nvimpager` value: "'
                .. config.nvimpager
                .. '". Valid values are: "tab", "split", "float", and "no".'
        )
        return
    end

because lua/r/config.lua already warns if there is an invalid option.

jalvesaq commented 2 months ago

I'd suggest split_h and split_v instead of split and vertical? Would mean making superseding split in favour of split_h, and maybe deprecating it after a while.

I agree, but since we are still in the first months of R.nvim and most people still seem to be using Nvim-R, we can simply change the option name without a deprecation period. Anyway, we also have to document the change in the Transitioning from Nvim-R section in the README.

jalvesaq commented 2 months ago

The function open_example() (lua/r/edit.lua) has to be adjusted too. Ideally, we would have a single function to open examples and open the documentation, but the user might want to see and edit an example while looking at R documentation, and because the example can be edited, we can't simply replace it with another example or documentation. They need to be in separate functions, but we have to replicate the same algorithm in both.

cswingle commented 2 months ago

Thanks for the comments. I'll revise a bit later today to change the option names, clean up the code, and add a short section to Transitioning explaining the options. In the section that actually does the vsplit I'm not doing any sanity checking of the terminal width/height, etc. I wasn't sure what the logic is in the horizontal split section so I didn't add anything similar to the new code.

jalvesaq commented 2 months ago

I think it's better to avoid any complicated algorithm as we have in Nvim-R. If the user chooses "vertical" he/she will get a vertical split. The goal of the horizontal split is to behave exactly as Vim/Neovim default help, as described by:

:help help
cswingle commented 2 months ago

I updated the options, removed the duplicate options checking, and added the list of current options in the Transitioning section of the README. I didn't modify the split_h code, which seemed to be working for me. split_v also works for me. I didn't test the section that opens a window for examples, but I did modify the code there. Let me know if anything else needs fixing/updating.

jalvesaq commented 2 months ago

After <LocalLeader>re over a function name, I get this error:

Vim(lua):E5108:
Error executing lua /home/aquino/src/R.nvim/lua/r/edit.lua:261: attempt to compare number with nil

The problem is this code:

        if config.nvimpager == "split_v" then
            local wwidth = vim.fn.winwidth(0)
            local min_e = (config.editor_w > 78) and config.editor_w or 78
            local min_h = (config.help_w > 78) and config.help_w or 78
            if wwidth < (min_e + min_h) then nvimpager = "split_h" end
        end

which use config variables that no longer exist. I fixed it.

cswingle commented 2 months ago

Thanks for fixing. I hadn't tested the examples function so I'm glad you did. I'll go read the manual to see what it does!

jalvesaq commented 2 months ago

Thank you for your contribution!