akinsho / toggleterm.nvim

A neovim lua plugin to help easily manage multiple terminal windows
GNU General Public License v3.0
4.21k stars 170 forks source link

feat: allow operator mapping to send to terminal #507

Closed BlueDrink9 closed 9 months ago

BlueDrink9 commented 10 months ago

First, make a correction allowing lua functions to get the correct visual range.

Then add checks to allow motions, allowing significant flexibility for how you send text. Examples in the readme show how to use it to do things like send the whole file.

BlueDrink9 commented 10 months ago

I took a crack at some more significant refactors that would tidy this code up a lot, because it's getting a bit convoluted having to handle all of the strings used to track the mode.

However, I ran into real issues maintaining backwards compatibility, because of what IMO was a strange interface decision for the REPL stuff. Having two different functions for visual selection vs visual lines, IMO, is pointless because vim already has a paradigm for handling that - v vs V. I looked at the github accounts for the people that have introduced/contributed to this repl code, but I couldn't find dotfiles to see how/if they use it.

I've read that you don't like having to maintain this section of the code, so my proposal would be introduce a breaking change and delete the visuallines command. I could then tidy the rest up a lot to always just use the visual or motion selection.

Another thing that seems unusual to me is that we handle sending block selections, which I can't imagine ever being useful. But that might just be a limitation of my own imagination, so might not be worth removing.

akinsho commented 10 months ago

@BlueDrink9 thanks for the contribution haven't had time to read the PR in detail but just wanted to mention it's on my list. In general seems like a good idea I'm open to though 👍🏾

BlueDrink9 commented 10 months ago

Just to clarify, this PR doesn't make any major refactors --- I was just bringing it up to potentially tidy things up in a 2nd PR

akinsho commented 9 months ago

Gonna merge this as it seems OK didn't get a chance to thoroughly test so will await feedback from people who use the main branch (unstable version)