[Vimperator] [Patch] Proposition for another hint system : f* and f!

[Martin Stubenschrott]

mahefa randimbisoa wrote:

> It adds an option hintvoid (default=!) which you can type after f to
> select only void elements.

I don't like that to be an option, rather use a variable :let g:hintcharkey
and :let g:hintformskey. Actually I think , and . would be better keys than
* and !, as the former should be on a non-shifted key on most layouts.

> This is the first time I'm sending a patch, I'm not sure the format is
> correct. (Realized it with TortoiseSVN, the only tool I know and have
> that can create the patch).


The format seems correct to me.

 
> Can one please test it? To resume, this is how it works:
> 
> At any time after you type f, you can type * to switch to
> alphabetic-driven hints, and after this, you type the letters shown in
> the hint to select the link.

That does not work at all on all sites which I have tested. It just beeps
when I type *, and shows "*" in the statusline.

> When you want to select void elements (links that have no text
> associated like blank textareas, blank input, inputbox, ...), you type
> ! after the f. Only void elements will be hinted and hints are formed
> with alphabetic characters only.


That works well.

 
> I'm not aware of some special coding guidelines and coding
> styles/preferences, and I will really appreciate some helps from the
> devs about how to do a proper patch.

All in all, it seems quite good, although i have not yet looked through it
in detail.

Some notes:

1.) Lines like that (i saw at least 2 occurrences) should be documented above, if possible.
Why 324, why 649, etc. or at least what it does.

+                    span.textContent = "" + (hn+9+10*parseInt((hn-1)/26)+324*parseInt((hn+649)/676)).toString(36);

2.) Missing asciidoc documentation about the new features.
3.) Add yourself to the AUTHORS file and put a small note about the new feature in the NEWS file.
4.) I don't like the usedVoid etc. names too much, why not just usedFormsKey, or something similar?
5.)
+                if (!usedStar)
+                    span.textContent = "" + (hintnum++);
+                else
It's better if (usedStar) ... else ...


Note that these are just quick observations, as i could not test the "*" behavior. When I get a patch
which works with that, I may find additional things, which do not work.

But for a first draft, the patch looks fine for me.

best regards,
--
Martin