EXPERIMENTAL visual mode from selection

[albertdev]

As requested in issue #511, a setting visualother / vo which enables syncing an Eclipse selection with Vrapper's visual mode.

I noticed that it immediately went wrong if you would run something like dd u. The reason is because the following happens:

• Vrapper deletes a line of text. This action goes into the undo history.
• The deletion is undone. Eclipse puts back the text and selects it.
• The new selection handler change will put Vrapper into Visual mode.
• Vrapper's history manager deselects the text because that's how Vim does it.

It turns out that if you clear the selection of the textwidget instead of the ITextViewer, no selection change listeners on the ITextViewer will be called. Result: the selection is empty but Vrapper still thinks it should be in Visual mode. I worked around that with the second commit.

EDIT: It might still do weird things when renaming stuff, or using Ctrl + Space for completing commands. Eclipse tends to select stuff at those times, rapidly switching modes and leaving Vrapper in a bad state.

[keforbes]

As mentioned in #511, I've created a build of this branch and put it on an "experiment" update site: http://vrapper.sourceforge.net/update-site/experiment

I'm concerned with the fact that @albertdev keeps finding little issues that this introduces (the textWidget vs. ITextViewer issue, the Ctrl+Space behavior). Since he found those issues within hours of opening this Pull Request, I'm hesitant. I'll let other people decide if this introduces too many new defects. Hopefully I'm just being overly paranoid and there won't be other issues with it. We'll see.

[albertdev]

Does my solution differ a lot from what you had in mind?

I did introduce an option to toggle it as a sort of escape-hatch, so if we change the default to "disabled" it could in theory be included in the unstable build without harm.

I only used it for about a day back in July, and it's then that I noticed those small things. Maybe I should try it again...

[keforbes]

No, your solution doesn't differ at all from what I had in mind. In fact, you took it one step further and made it configurable. But I wasn't comfortable with the solution in the first place, it isn't your implementation that concerned me.

This might just be the way I use Vrapper, but to me, this fixes something I never use at the risk of destabilizing features I do use. That's why I don't want a "half-way" solution. I don't want to destabilize Ctrl+Space or visual mode just so using the Eclipse refactor functionality is slightly less annoying. Of course, @haridsv would disagree with me since he said it's the predominant issue for him. That's why I don't want my workflow to dictate whether this Pull Request is a valid fix. To me this is an annoyance; to others it's a major issue.

[haridsv]

I am going to try the build from experiment site and see if the added issues are going to be a bigger deal than the missing sync.

[haridsv]

I did some limited testing today and it worked very well for renames and Next (Ctrl+.) and Previous (Ctrl+,). It didn't however change the behavior for create method. To reproduce, add the below code in an existing Java method:

someNoneExistingMethod(10);

and use quickfix to create this method. You should then be able to enter a new name for it by pressing s. However, tab works to go from one id to another and be in the visual mode, so it is definitely an improvement. I did notice one unexpected issues. After I pressed multiple tabs to reach the parameter type and selected a different type from the drop down (e.g., long instead of the default int), tabbing to the next id resulted in the wrong visual mode.

I can also confirm that after Ctlr+Space, the visual mode is not set correctly, so this patch doesn't fix this scenario.

Please note that I am out most of the rest of the week, so may not test it much more than this for now. Thanks to @albertdev and @keforbes for getting the patch this far.

[haridsv]

I hit a weird bug that is most likely the side effect of this patch. If you use "Extract to local variable" followed by <Tab>'s to select the new id, the s command substitutes more than just what is currently selected.

[keforbes]

If you disable the fix by setting :set novisualother does the problem go away?

[haridsv]

@keforbes Sorry about the late response, but I can confirm that the issue goes away with :set novisualother.

[keforbes]

@albertdev, what do you think about this Pull Request? Do you think it's still worth merging in?

[albertdev]

I've been thinking about it, but it's still not ready for stable.

In the mean time, I did think about implementing select mode. Basically, it behaves like Notepad or Eclipse use their selections: you can extend using Shift + Left / Right, pressing an alphanumeric key replaces the selection with that key and pressing <C-G> switches to Visual mode or back.

For Vrapper, it's a mode very like Insert mode (i.e. we let pretty much any character pass straight to the editor). For Eclipse, this is just its native selection mode.

[albertdev]

I'm going to close this pull request... I'm not any closer to fixing this prototype.

The idea about select mode can better be moved to another issue should there be interest, though I guess it will just confuse users when they try to invoke a command and replace the selection with that character instead.

I have been thinking about doing something with Command Listeners though, it might be possible to recognize that a command id is on a white-list after which we reset Vrapper's selection state.

[albertdev]

I've been working on this once more after @mmbradle told me in #674 that you could actually swap between normal and visual mode without much fuss.

I fixed @haridsv's problem with the s command back in April with commit c1c8c56f0bc80dfb5760de883f2137afa10bfb45 - turns out that Vrapper was caching something it shouldn't be caching.

The other problem @haridsv reported with Ctrl + Space was that our "exit link mode" feature would clear the selection and go to Normal mode after auto-complete triggered a selection AND link mode. To work around this issue I now move from Insert to Select mode, which maps closer to the way you normally work with auto-complete (Eclipse proposes a value and selects it so you can immediately overwrite it by mere typing). Select mode is then excluded from the "exit link mode" handling and hence everything just works.

What this prototype doesn't yet do:

• Give you any choice in the matter of picking whether Normal mode / Insert mode go to Visual mode or Select mode
• It doesn't yet handle Shift + Arrows to start it from Normal mode (though it works for Insert)
• You can't switch from Select mode to Visual mode (Vim offers Ctrl + G to do so)
• Certain Eclipse actions like "Quick Outline" will select a variable / function name after picking it from the menu, this might surprise you by staying in visual mode.

It does give hope that we could finally integrate Eclipse's selections with Vrapper.

[pedrosans]

imo Vrapper should always go to visual mode when a piece of text is selected. I tried Vrapper with this branch and it was really counter intuitive to be inside select mode, that I would normally go into only by configuring 'selectmode' option or 'g_h' from normal mode. I tried to change SelectionVisualHandler so it would only enter visual mode, but noticed the problem:

• select a variable using the 'Select Enclosing Element' command
• start the refactoring by using the 'Rename - Refactoring' command
• 'c' VIM command + the new variable name + ENTER

as a result, I got the renamed variable fully selected but Vrapper in normal mode.

I think we can get a partial solution with Command Listeners as @albertdev commented. Will give it a try in https://github.com/pedrosans/vrapper/tree/visual-mode-sync and share the results next.

[albertdev]

@pedrosans You just reminded me that I had some experimental code with command listeners sitting in a local branch, and it was never backed up to GitHub. Had you asked before I could have shared it earlier - though it is silly of me that I didn't upload it as backup...

You can now find it at https://github.com/albertdev/vrapper/commits/EXPERIMENTAL-eclipse-motion-textobjects - it is currently based on the old experimental code in this Pull Request so that I could compare the visualother setting with the new code.

It's out of date and might break in interesting places, but it did include a configuration mechanism through which Vrapper plugins could register extra commands (e.g. a Python plugin registering python-only commands). I also started on some things like repeating Eclipse commands through the . key.

[albertdev]

@pedrosans By the way: we have a Gitter channel. If you want to discuss things you can also ask there.