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

Paste vs bracketed paste vs source; echo vs silent. #23

Closed
jalvesaq opened this issue Feb 9, 2024 · 35 comments
Closed

Paste vs bracketed paste vs source; echo vs silent. #23

jalvesaq opened this issue Feb 9, 2024 · 35 comments

Comments

@jalvesaq
Copy link
Member

jalvesaq commented Feb 9, 2024

In the past, when sending multiple lines to RConsole, if the total number of bytes was over 4 Kb, the text was incompletely pasted by Tmux (required to run R in an external terminal on Unix). To test if the problem persists, I converted a PDF book to txt and then converted each line into a cat():

%s/\(.*\)/cat("\1\\n")/

The result was a file with 500 Kb.

There was no problem sending all the lines concatenated with "\n" directly to R if it was running in Neovim's built-in terminal. Also, there was no problem sending the code to Tmux.

However, a bracketed paste didn't work with Neovim's built-in terminal or Tmux.

Considering these results we may consider that we no longer need to source multiple lines if running R on a local Linux system. However, it is prudent to keep the option to source code at least in some circumstances, for example, when running R in a remote machine. We also have to keep the bracketed paste option for Python code in Quarto documents.

Although it's possible, I can't see any usefulness of pasting hundreds of lines on the R Console since the code will scroll up and be out of sight anyway. So, I'm trying to figure out what should be the default behavior. Perhaps: Paste the code directly if the number of lines is below max_lines_to_paste; otherwise, source it.

Another problem is the number of key bindings created by default: four for each source operation, combining "echo" and "movement". I think we can get rid at least of the "echo" option because echo=TRUE can be added to source_args.

@PMassicotte , @she3o, any suggestions?

@PMassicotte
Copy link
Collaborator

I like the idea of max_line_to_paste. We could also echo like the first n and the last n lines of the block. So, if I understand correctly, we could simply send the lines to the console without sourcing a temp file (except for some use case you describe)?

@jalvesaq
Copy link
Member Author

jalvesaq commented Feb 9, 2024

We could also echo like the first n and the last n lines of the block.

It's a good idea, but I don't know how to implement it with base::source().

So, if I understand correctly, we could simply send the lines to the console without sourcing a temp file (except for some use case you describe)?

Yes, except if the number of lines is > max_lines_paste. I suggest a default value close to 20 for this option. Of course, users would be free to change it to any value between 1 and inf.

@jalvesaq
Copy link
Member Author

jalvesaq commented Feb 9, 2024

We could also echo like the first n and the last n lines of the block.

Actually, this could be done inside nvimcom:::NvimR.source()

@PMassicotte
Copy link
Collaborator

PMassicotte commented Feb 10, 2024

Should send functions work at the moment? I am trying to use the tmp-R-Nvim and I keep getting R is not ready yet, coming from this line

https://github.com/jalvesaq/tmp-R-Nvim/blob/587be7749934431b5ab13e2745eeaa4f14560441/lua/r/send.lua#L65

rf indeed opens the R terminal, but when I try to send a function I get the R not ready.

@jalvesaq
Copy link
Member Author

I was testing the external terminal, and introduced this bug for the built-in terminal. I will fix it in a couple of minutes.

@PMassicotte
Copy link
Collaborator

No rush :)

@jalvesaq
Copy link
Member Author

I can't replicate the bug.

@jalvesaq
Copy link
Member Author

Could you try this:

echo 'remove.packages("nvimcom")' | R

Maybe nvimcom needs to be rebuilt...

@PMassicotte
Copy link
Collaborator

I removed nvimcom and it re-install when I open a R file in nvimr. But I still have that same issue.

Peek 2024-02-10 13-08

@PMassicotte
Copy link
Collaborator

WIll investigate and report back.

@PMassicotte
Copy link
Collaborator

Actually, I am getting some lua errors. Do you get some if you type :messages

image

@PMassicotte
Copy link
Collaborator

Looks like it is coming from exec_stdout_cmd

@jalvesaq
Copy link
Member Author

When an environment variable doesn't exist, the vim.env.VAR is nil...
I will grep 'vim.env.*\.\.' and fix the cases when the environment variable might not exist.

@jalvesaq
Copy link
Member Author

If indeed the problem was the environment variable being nil, it should be fixed now.

@PMassicotte
Copy link
Collaborator

Super, working perfectly fine!

@PMassicotte
Copy link
Collaborator

The echo/silent and down/stay are inverted in function call:

https://github.com/jalvesaq/tmp-R-Nvim/blob/46ef896d83331879ec2685628688f2a04f114a50/lua/r/maps.lua#L150-L154

https://github.com/jalvesaq/tmp-R-Nvim/blob/46ef896d83331879ec2685628688f2a04f114a50/lua/r/send.lua#L172-L192

so m is equall to either echo/silent.

We can fix that later when we decide what to do with the echo/silent options.

@jalvesaq
Copy link
Member Author

We can fix that later when we decide what to do with the echo/silent options.

What is your opinion on this? I prefer to eliminate the "echo" parameter from the send functions because users can set the value of source_args as ", echo=TRUE". It's redundant to have both maps for echoing what is being sourced and the option to do this with source_args. I prefer to simplify the functions and reduce the number of key bindings. But if you like the parameter we can keep it.

@jalvesaq
Copy link
Member Author

Super, working perfectly fine!

If you do git pull now the Object Browser will auto start faster because I removed the 1 second delay on Unix.

@PMassicotte
Copy link
Collaborator

We can fix that later when we decide what to do with the echo/silent options.

What is your opinion on this? I prefer to eliminate the "echo" parameter from the send functions because users can set the value of source_args as ", echo=TRUE". It's redundant to have both maps for echoing what is being sourced and the option to do this with source_args. I prefer to simplify the functions and reduce the number of key bindings. But if you like the parameter we can keep it.

Make total sens. I up to remove echo.

@PMassicotte
Copy link
Collaborator

Super, working perfectly fine!

If you do git pull now the Object Browser will auto start faster because I removed the 1 second delay on Unix.

Oh this is nice. Open a nanosecond after the terminal. Tx!

@jalvesaq
Copy link
Member Author

So, please, feel free to remove the "echo" parameter if you have time.

@PMassicotte
Copy link
Collaborator

all right, will do it right now

@jalvesaq
Copy link
Member Author

I uploaded some bug fixes in many files. So, please, do git pull before pushing new changes. I intended to do a pull request, but I forgot to create a branch and pushed the changes to the main branch.

@jalvesaq
Copy link
Member Author

I'm implementing the option max_paste_lines...

@PMassicotte
Copy link
Collaborator

Great! The project is taking shape!

@jalvesaq
Copy link
Member Author

I already tried most features, and they are working. One problem is some key bindings not being activated. When the same function is mapped twice, one with argument false and the other with true, only the second one is active, as we can see with :nmap. This happens at least with SendParagraph, and SendSelection.

@PMassicotte
Copy link
Collaborator

Meaning that

    -- Paragraph
    create_maps("ni", "RSendParagraph",   "pp", "<Cmd>lua require('r.send').paragraph(false)")
    create_maps("ni", "RDSendParagraph",  "pd", "<Cmd>lua require('r.send').paragraph(true)")

Is only mapping the last one?

@jalvesaq
Copy link
Member Author

Thanks for looking at that!

I found the problem: it was my configuration! I was setting custom key bindings and they were preventing maps.lua to do its job.

My default mapping to send lines of code to R, <Enter> is also preventing me from using the LSP quickfix list. I will create a new hook to overcome this problem.

@PMassicotte
Copy link
Collaborator

oh cool!

@jalvesaq
Copy link
Member Author

My configuration now is (commented lines ommited):

return {
    dir = '~/src/tmp-R-Nvim',
    config = {
        R_args = {'--quiet', '--no-save'},
        hook = {
            after_config = function ()
                vim.api.nvim_buf_set_keymap(0, "n", "<Enter>", "<Plug>RDSendLine", {})
                vim.api.nvim_buf_set_keymap(0, "v", "<Enter>", "<Plug>RSendSelection", {})
            end
        },
        -- user_maps_only = true,
        auto_start = 1,
        objbr_auto_start = true,
    },
    lazy = false
}

@PMassicotte
Copy link
Collaborator

Great, I will try that when I am back at home in few hours.

@jalvesaq
Copy link
Member Author

I have had to create some maps conditionally:

    {
        dir = '~/src/tmp-R-Nvim',
        opts = {
            R_args = {'--quiet', '--no-save'},
            hook = {
                after_config = function ()
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rq', '<Plug>RClose', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>ss', '<Plug>RSendSelection', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>sp', '<Plug>RDSendParagraph', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rl', '<Plug>RListSpace', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rm', '<Plug>RClearAll', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rp', '<Plug>RObjectPr', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rn', '<Plug>RObjectNames', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rt', '<Plug>RObjectStr', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rv', '<Plug>RViewDF', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>ra', '<Plug>RShowArgs', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rh', '<Plug>RHelp', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rs', '<Plug>RSummary', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>ro', '<Plug>RUpdateObjBrowser', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>r=', '<Plug>ROpenLists', {})
                    vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>r-', '<Plug>RCloseLists', {})
                    if vim.o.syntax ~= "rbrowser" then
                        vim.api.nvim_buf_set_keymap(0, "n", "<Enter>", "<Plug>RDSendLine", {})
                        vim.api.nvim_buf_set_keymap(0, "v", "<Enter>", "<Plug>RSendSelection", {})
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>sb', '<Plug>RSendMBlock', {})
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>rf', '<Plug>RStart', {})
                    end
                    if vim.o.filetype == "rmd" or vim.o.filetype == "quarto" then
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>kn', '<Plug>RKnit', {})
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>kr', '<Plug>RMakeRmd', {})
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>gn', '<Plug>RNextRChunk', {})
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>gN', '<Plug>RPreviousRChunk', {})
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>sc', '<Plug>RSendChunk', {})
                        vim.api.nvim_buf_set_keymap(0, 'n', '<LocalLeader>sh', '<Plug>RSendChunkFH', {})
                    end
                end
            },
            user_maps_only = true,
            -- clear_line = false,
            auto_start = 1,
            -- bracketed_paste = false,
            objbr_auto_start = true,
            term_title = "tmux",
            -- external_term = "foot",
        },
        lazy = false
    },

@PMassicotte
Copy link
Collaborator

You have to do it to avoid duplicated map? On my side all works fine (maybe I missed something)

@jalvesaq
Copy link
Member Author

The main reason was that when I pressed <leader>gd over a function name in a Lua buffer, the LSP opened a quickfix window with two lines and, then, when I pressed <Enter> nothing happened. The problem was that I mapped <Enter> to send lines to R, and the map was active in all buffers. The hooks are very cheap to add; just 2 lines of code: one in the table config and the other where the function should be called.

@PMassicotte
Copy link
Collaborator

Good to know!

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

No branches or pull requests

2 participants