[Vimperator] [patch] Mappings refactoring

[Martin Stubenschrott]

On Fri, May 25, 2007 at 12:33:16PM +0300, Виктор Кожухаров wrote:
> Hi

Hi, nice you found some time to do work on vimperator again :)

> I've done some work on the mappings. The lowdown is that I moved them a
> new object, under V.prototype.mappings.main (since there should be
> V.prototype.mappings.user for user defined mappings) as a multiarray
> with the first dimension being the mode.

I am not convinced by that as I want to objectize mappings (like
commands).

So my idea was to have two classes like:

function Mapping(cmd, action, extra_info) // extra info is a hash
{
	//cmd and action are required
	this.command = cmd;
	this.action = action;
	// all others are optional
	if (extra_info)
	{
		if (extra_info.usage)
			this.usage = extra_info.usage;
		if (extra_info.help)
			this.help = extra_info.help;
			...
	}
	else
	{
		// do some heuristics where possible for non-specified args
	}
}


function Mappings()
{
	// private
	var mappings;
	var user_mappings;

	// public
	// would be called like:
	// vimperator.mappings.add(new Mapping("/", vimperator.search.openDialog,
	//            { usage: "moo",
	//              help: "find it"}
	this.add(mapping);

	// note that mapping is not a object of the mapping class but a
	// string, and command is any string
	// because a user should be able to map:
	// :map x :open foo.com<Return>
	// then mapping="x" and command=":open foo.com<Return>"
	// this should internally create a Mapping object, like this:
	// { user_mappings.push(new Mapping(mapping, function() {vimperator.commands.normal(command)}); // v.c.normal will be the ex command :normal
	this.addUserMapping(mapping, command);

	// later maybe:
	this.removeUserMapping(mapping); // only for user mappings, main
	                                 // mappings never should be removed

	// TODO: how to handle incomplete mappings (add a also_incomplete
	// param and return an array of partial commands if set?)
	// also this.get() should be an intelligent function and aware of
	// main and user mappings and return a Mapping() object which is
	// then ready to be called
	this.get(mapping_str);
}

Well this may be an incomplete api, but that's how I want it.
I am sorry, you did the work, and I can't commit it in this stage, but
for bigger things like that, it's probably better to discuss the design
on the ML before doing the implementation.
But I don't have time until monday anyway to do much coding, so if you
have time and want to hack around, the repo shouldn't change that much.

> I also added a new maptype
> field to the normal mappings, so that we could eventually support
> mappings with parameters (I need this to implement the marks mappings)
> or motion mappings.

Ah great, i thought about how to add pending commands like needed for
macros or marks anyway, didn't find a good solution. but this is, great.

Put please make the modes a public hash like:
    this.modes = { // actually not private, but Firefox complains if this doesn't come first
        // main modes
        NONE:             0,
        NORMAL:           1 << 0,
        INSERT:           1 << 1,
	}
... in vimperator.js instead of global consts (which I want to get rid off)

> also while i was doing that, i noticed that the new search is mapped
> under 'g/'. why is that? I added the '/' as a map to it, since I didn't
> even know we had a new search until i looked at it. so why is the 'g/'
> even necessary?

Because the search is buggy, and can't be enabled by default until the
only show-stopping bug is fixed:

Go to www.orf.at (or I think any other site with frames), and search
with g/, you will see, it doesn't work and even breaks the find for
other tabs. Interessting catch:
If you serach with / (the default find of firefox) first on this page,
then you can search ONCE with g/ and then you have to use / again before
you can use g/ again.

I and waawaamilk from IRC haven't found the problem yet, therefore it
stays disabled for now.

There are some other minor issues, but this one bug has to be ironed
out, before I will change g/ to /, see the wiki for other thigns:
http://vimperator.cutup.org/index.php?title=Search

PS: CC'ing to the ML, because the mapping design could be of interest to
others as well.