R-nvim / R.nvim

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

Improve completion of methods arguments #82

Closed jalvesaq closed 3 months ago

jalvesaq commented 3 months ago
jalvesaq commented 3 months ago

This pull request should be tried with the branch improve_methods_args from cmp-r.

Building an args_ file was about three times slower than building the corresponding objls_ file. After some optimization, it's now about 5 times faster than before. The big improvement in the performance was due to avoiding calling help() just to get the directory where the package was installed. Now that it's much faster, it is being built right after the objls_ file. This simplifies the code, which now has about 260 fewer lines.

PMassicotte commented 3 months ago

oh let me try it!

PMassicotte commented 3 months ago

image

Thas is nice!

PMassicotte commented 3 months ago

Are we missing some parameters?

image

jalvesaq commented 3 months ago

correlation is for summary.lm, not for print.summary.lm

PMassicotte commented 3 months ago

oh wow, I still need coffee this morning :)

I have to make some change to a project, I will report back in 1 h ( I will use your PR)

jalvesaq commented 3 months ago

In this example, the R documentation is shared by two functions and the Arguments section combine all arguments of the two functions.

PMassicotte commented 3 months ago

This feels very fast already

jalvesaq commented 3 months ago

The completion by rnvimserver + cmp-r is almost instantaneous. The only slowness is making the cache files when the library is loaded for the first time.

PMassicotte commented 3 months ago

Well, I have worked roughly 1 hour and it works perfectly fine. Speed is not comparable as it was before. You did a very nice job here :) I do not see any problem at mergin this :+1:

jalvesaq commented 3 months ago

I ran it with Valgrind and there was no invalid read or write in the C code. So, I believe that it's safe to merge. If any bugs appear, we will fix them.

PMassicotte commented 3 months ago

I have tried it also on a Window machine and no issue.

jalvesaq commented 3 months ago

Thank you!

PMassicotte commented 3 months ago

Just need to merge the cmp branch

jalvesaq commented 3 months ago

I'll do it... Thanks for remembering!