[Vimperator] [patch] Mappings refactoring

[Martin Stubenschrott]

On Fri, May 25, 2007 at 02:32:12PM +0300, Виктор Кожухаров wrote:
> >>From what I understand, Mapping is a function to add users mappings?

No, Mapping() is not a function to add user mappings, it is the base class
of any created Mapping() object, which is then added to the
Mappings() class which keeps track of all (also user-) mappings.

> But why not just have a method in the Mappings object that does that,
> like : add: function(mode, cmd, action, extra_stuff), and remove: ...
> And with the api you propose, where do the default maps get stored? My
> proposition doesn't look that different. I still add a mappings
> object, but I haven't implemented the add/remove/get methods yet. I
> think that add/remove should only work on user mappings, since we have
> to have a way to return to the default mappings. I think vim itself
> never overwrites the default mappings, but looks in the user mappings
> first.

var mappings = new Array(); // this would be the non-end-user-modifiable
                            // array of default Mapping objects

The big advantage of having a seperate Mapping() class instead of a
multidimensional array is that you can have proper names for the
properties. So a common usage scenario could look like this:

// v.m.get() takes care of first looking into user maps and then into
// default maps
var mapping = vimperator.mappings.get("/");
if(mapping)
{
	alert(mapping.usage);
	mapping.execute();
}

This is much cleaner than the current approach of having:
mapping[COMMAND].call(this,...); for me

Also you are right, AFTER vimperator is initialized no mapping should
ever be added to the default map, but only with
vimperator.mappings.addUserMapping("x", "...");

I think we have a similar idea, but you want:
add: function(mode, cmd, action, extra_stuff)
and I want:
add: function(mapping) // mapping is a new Mapping(mode, cmd, action)

Well, one could archieve the later with the first, by creating a new Mapping
inside the add() function, but is this a better API?

> I've been having a quick look inside the find.js file, and so far I
> think that's not the correct approach. Most of the code can be made
> redundant by just reusing useful function from gFindBar. Reusing
> gFindBar.highlightDoc would obsolete a lot of the code, since that
> function does the searching and highlight everything. We can also reuse
> the finding code itself, or just take it and add regex support to it.

Hmm, haven't looked into that myself, as the code is nearly completly
stolen from conkeror.