Re: VIM Mode

162 views
Skip to first unread message

Irakli Gozalishvili

unread,
Feb 10, 2011, 5:22:03 AM2/10/11
to ace-in...@googlegroups.com, Fabian Jakobs, jordant...@gmail.com, Tim Taubert


On Thu, Feb 10, 2011 at 10:55, Tim Taubert <tim.t...@gmx.de> wrote:
Hey, I commented inline...


On 10.02.2011 10:46, Irakli Gozalishvili wrote:
Hi Everyone,

Please see my comments inline:


   On 10.02.2011 10:09, Fabian Jakobs wrote:

       Good morning,

       all three of you have contributed extended vim support. This is
       great
       but also represents a dilemma for me - which one to pull? I'd
       like to
       merge all three of them but unfortunately the approaches are not
       compatible. However I came to the conclusion that the vim mode
       should
       be part of Ace to give a clear path for future vim mode
       improvements.


This is great I hope we'll start collaboration on one VIM mode soon!!
Still Fabian I really think that VIM mode should be a plugin even if it
will move to ace repo, also IMO it's better to not pull it into ace. I
think official vim plugin or a git sub-module will be better.

+1



       I'd like to ask each of you to look at the code of the other two and
       discuss how the best solution for a vim mode in Ace could look like.
       I'll try to moderate.


I have not looked into details of other two implementations yet, but it
seems jordanlewis's implementation is pretty basic and easily can be
merged with the mine all the missing bindings can be just added to the:
https://github.com/Gozala/vice/blob/master/lib/keyboard.js

Tim's approach looks interesting, I have to look deeper but I have some
comments already:

1. It has a different license then rest of the ace, I'm not really into
whole licensing stuff but I'd prefer if all of the ACE was licensed with
the same.

I'm ok with ACE's default license, too. I just picked my default one.


Great !!
 

2. If I read code correctly it defines command 'custom' that takes
buffer input and then translates it to the, VIM operations. It's neat
but I think that defining each command separately has a benefit of code
re-usability through
CLI or in other keyboard bindings, also that is a reason I took that
path. Maybe combination of both will be the best.

Registering "real" ACE commands for all those operations is a good idea. Although that'll probably be a lot :)

Yeah there's will be a lot of them for sure :P Also to make myself clear I think that all commands should live in main ACE repo while keyboard mapping etc can be in plugin.
 


I'll post more once I'll have a chance to look and test more, also I
think we should move this discussion to the mailing list.

Meanwhile you can test my code by cloning my repo
https://github.com/Gozala/vice and updating submodules.
Alternatively you can play with this firefox's extension that has VIM
mode by default: https://github.com/downloads/Gozala/sky-edit/

Damn, I started a similar FF extension some days ago :)


Well that's great we can collaborate then, no ?
 


       Does this make sense to you?

       Best,
       Fabian

       https://github.com/Gozala/vice/tree/master/lib
       https://github.com/ttaubert/ace/tree/ac3f1021f8bfee3e5fb8a1a55a494e86bb4291fe/lib/ace/keyboard
       https://github.com/jordanlewis/ace/commit/a9f8fbc2eebe996065255b14818e94b37fc55e69

       P.S.
       @jordan are you on the mailing list
       <http://groups.google.com/group/ace-internals>? Then we could
       discuss
       it there.



Irakli Gozalishvili

unread,
Feb 10, 2011, 5:30:32 AM2/10/11
to ace-in...@googlegroups.com, Fabian Jakobs, jordant...@gmail.com, Tim Taubert


On Thu, Feb 10, 2011 at 10:51, Tim Taubert <tim.t...@gmx.de> wrote:
I just had a quick look at the two so please don't feel offended if I got something wrong :)

@jordanlewis

This is a nice quick fix to get some more VIM commands working in ACE but using the normal StateHandler we'll never get to real VIM mode support (think of visual mode, registers, commands with ":", replay and so on). "gotolinestart", "gotoword*", "gotoright/left" also behave quite different in VIM and the meaning of "word" should imho be configurable.

+ 1 Please don't feel offended :) In fact my first version looked pretty similar :)
 

@Gozala

I really like your plugin approach and it's obvious that you're way more into ACE than I am :) You're also using the StateHandler which is imho not sufficient for parsing VIM commands.


Yeap I though of that as well, but hoped that StateHandler can be improved on the way :) 
 
VIM commands basically consist of the operation, the counter and a modifier. So these are all the same: "2dd", "d2d", "dj". It is even possible to have commands like "3d3d" which is equal to "9dd" or "d9d". Not to forget "dw", "d$", "d0", "d^", "dG", "dgg", etc. That's why I decided to write a specialized "parser" for these commands. Once we support registers it's actually no big deal to add them afterwards.

And you are way more advanced in VIM :) I did not knew that also I was expecting that I'll hit the limit with a current approach at some point :) Do you think think state handler can be improved to handle all of that ?


Movement in VIM is very important so we should stick to default behaviour with word movement but of course with "^" and "0", too.

@ttaubert

Yeah I'm addressing myself ;) I'm not quite happy with the structure of my code at the moment. I thought a lot about the movement and how VIM commands should be parsed to get everything right. So there's a lot of room for refactoring and cleanup. But as a whole that seems to work for me quite good although there are still lots of important commands missing.


Cheers :)

Tim


On 10.02.2011 10:09, Fabian Jakobs wrote:
Good morning,

all three of you have contributed extended vim support. This is great
but also represents a dilemma for me - which one to pull? I'd like to
merge all three of them but unfortunately the approaches are not
compatible. However I came to the conclusion that the vim mode should
be part of Ace to give a clear path for future vim mode improvements.
I'd like to ask each of you to look at the code of the other two and
discuss how the best solution for a vim mode in Ace could look like.
I'll try to moderate.

Fabian Jakobs

unread,
Feb 10, 2011, 5:41:24 AM2/10/11
to Irakli Gozalishvili, ace-in...@googlegroups.com, jordant...@gmail.com, Tim Taubert
Hi,

this is what I get from your conversation

1. The keyhandler should be a separate project
2. The vim-mode will be pulled in by Ace as a submodule
3. All commands should go into Ace
4. It will probably be based on Tim's code

Do you agree? I'd be fine with this approach. My suggestions for the
governance of the vim-mode project would be:

1. I create ajaxorg/ace-vim on gh
2. I give each of you commit rights to this repo
3. Someone of you creates a pull request which integrates the
submodule with Ace (I can assist with this)
4. It would be great if one of you takes the responsibility for this
repo and can act as contact person for me
5. We remove the vim.js from Ace

These are my ideas. What do you think?

Best,
Fabian

Irakli Gozalishvili

unread,
Feb 10, 2011, 8:08:56 AM2/10/11
to Fabian Jakobs, ace-in...@googlegroups.com, jordant...@gmail.com, Tim Taubert, julian....@googlemail.com
CC-ing Julian, as he worked on state handler and other keyboard internals for skywritter / ace and may be helpful.


On Thu, Feb 10, 2011 at 11:41, Fabian Jakobs <fabian...@web.de> wrote:
Hi,

this is what I get from your conversation

1. The keyhandler should be a separate project

Yeap I think we all agree with that!
 
2. The vim-mode will be pulled in by Ace as a submodule

I think adding even more dependencies to Ace may not be the best idea, in fact it's already quite complicated. At the moment my plugin is pretty much independent and can be easily added to ace in case if necessity just by clonening it in the supports folder, I tried few other options and this worked best for me, so I'd suggest to keep that model. Also either on the wiki or ace readme we should mention the official vim mode plugin.

3. All commands should go into Ace

Sounds good to me!!
 
4. It will probably be based on Tim's code

Do you agree? I'd be fine with this approach. My suggestions for the
governance of the vim-mode project would be:


I really like name vice which is vim + ace, and if you guys don't mind much, I'd prefer keeping it. Also I'd suggest integrating Tim's and Jordan's code into vice and have an official fork on ajaxorg/vice
 
1. I create ajaxorg/ace-vim on gh
2. I give each of you commit rights to this repo

I don't think commit rights are necessary, working on forks is good enough IMO also we can benefit from reviewing each others code before checking that in.
 
3. Someone of you creates a pull request which integrates the
submodule with Ace (I can assist with this)
4. It would be great if one of you takes the responsibility for this
repo and can act as contact person for me

Probably we can have someone with push rights to the ajaxorg/{{plugin-name}}. It any case I'd love to have a pull requests for reviewing each others code.
 
5. We remove the vim.js from Ace

 
+1

These are my ideas. What do you think?


One important thing we have to decide is how to go about merging our projects. I see few options:

  1. Tim's custom handler can be hooked as a fallback handler through `match` function:
    https://github.com/Gozala/vice/blob/master/lib/keyboard.js#L163-166

    Jordan's work can be amended to the keyboard handler.

  2. Merging Tim's custom handler with a state handler and improving (merging rest with vice ?), also merging adding Jordan's handlers to existing map of handlers.

  3. Switching over to the Tim's version.

I'd suggest to go with 1-st and as a next step transition to 2-nd. That being said I'm not entirely sure if that is possible at all, I need to get more familiar with Tim`s code and would love to read how VIM does all of that.

Are there any other options / ideas ?

Once we support registers it's actually no big deal to add them afterwards.

Not sure I follow can you give some more details ?

 
Best,
Fabian

Fabian Jakobs

unread,
Feb 11, 2011, 2:45:26 AM2/11/11
to ace-in...@googlegroups.com, jordant...@gmail.com, Tim Taubert, julian....@googlemail.com
Hi,

>> this is what I get from your conversation
>>
>> 1. The keyhandler should be a separate project
>
> Yeap I think we all agree with that!
>
>>
>> 2. The vim-mode will be pulled in by Ace as a submodule
>
> I think adding even more dependencies to Ace may not be the best idea, in
> fact it's already quite complicated. At the moment my plugin is pretty much
> independent and can be easily added to ace in case if necessity just by
> clonening it in the supports folder, I tried few other options and this
> worked best for me, so I'd suggest to keep that model. Also either on the
> wiki or ace readme we should mention the official vim mode plugin.

The reason I want to bind it tighter to Ace is to increase visibility.
There seems to be a high demand for a good vim mode and I would like
Ace to have out of the box vim support. I would like to take the same
approach as we do now with pilot and cockpit. I don't think this
complicates the setup since the steps to get Ae running remain the
same.

> I really like name vice which is vim + ace, and if you guys don't mind much,
> I'd prefer keeping it. Also I'd suggest integrating Tim's and Jordan's code
> into vice and have an official fork on ajaxorg/vice

Fine with me. Also I wouldn't mind giving you push rights to ajaxorg/vice.

>>
>> 1. I create ajaxorg/ace-vim on gh
>> 2. I give each of you commit rights to this repo
>
> I don't think commit rights are necessary, working on forks is good enough
> IMO also we can benefit from reviewing each others code before checking that
> in.

Also agreed.

> One important thing we have to decide is how to go about merging our
> projects. I see few options:

I have no opinion on that. I think you can sort it out :)

Is everyone fine with promoting Irakli's vice to the oficial vim mode for Ace?

Best,
Fabian

Irakli Gozalishvili

unread,
Feb 11, 2011, 5:29:14 AM2/11/11
to ace-in...@googlegroups.com
Thanks Fabian,

Keep in mind that both of the fixes where just workarounds, one just changes z-index of cursor layer in normal mode so that cursor does not hides text under it. The change just reverts julians change which probably breaks something else :(

Regards
--
Irakli Gozalishvili
Web: http://www.jeditoolkit.com/
Address: 29 Rue Saint-Georges, 75009 Paris, France


On Fri, Feb 11, 2011 at 08:47, Fabian Jakobs <fabian...@web.de> wrote:
Thanks

> Also I identified two regressions that breaks some stuff in VIM mode:
>
> https://github.com/ajaxorg/ace/commit/be6646ba3555edc80ac1d872f3b030856e66fe10
> https://github.com/ajaxorg/ace/commit/ffada5c7b2f97240e890e4c6a608a26df1fcd5d1
>
> I made a quick fixes for those and they are here (so in case you play with
> vice you want to pull my vice branch of ace and link it).

I have pulled both fixes from your branch.

Fabian Jakobs

unread,
Feb 11, 2011, 6:55:59 AM2/11/11
to ace-in...@googlegroups.com
I know. I can live with a breaking IE since I need to do a proper IE
debugging session anyways.

> --
> You received this message because you are subscribed to the Google Groups
> "Ace Internals Dev" group.
> To post to this group, send email to ace-in...@googlegroups.com.
> To unsubscribe from this group, send email to
> ace-internal...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/ace-internals?hl=en.
>

Reply all
Reply to author
Forward
0 new messages