Hey, I commented inline...+1
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.
I'm ok with ACE's default license, too. I just picked my default one.
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.
Registering "real" ACE commands for all those operations is a good idea. Although that'll probably be a lot :)
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.
Damn, I started a similar FF extension some days ago :)
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/
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.
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.
@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.
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.
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.
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
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?
Once we support registers it's actually no big deal to add them afterwards.
Best,
Fabian
>> 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
Thanks
I have pulled both fixes from your branch.
> 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).
> --
> 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.
>